Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(app): rename error recovery "hooks" dir to "utils" #15449

Merged
merged 4 commits into from
Jun 18, 2024

Conversation

mjhuff
Copy link
Contributor

@mjhuff mjhuff commented Jun 18, 2024

Overview

A lot of the Error Recovery hooks used as utils are dependent on non-ER-hooks used as utils, and instead of making some weird cross-dependency between hooks that act like utils in a hooks directory and utils that are only used by hooks in a utils directory, I think it makes sense to keep everything in a "utils" directory and call it a day.

I'm open to thoughts/comments/concerns. There are a good number of utils in "utils", but if it's any consolation, we are most likely done adding any utils for MVP at this point (well, maybe there will be a couple more for good measure, I don't know yet).

Test Plan

  • CI passes.

Risk assessment

none

@mjhuff mjhuff requested a review from a team June 18, 2024 02:24
@mjhuff mjhuff requested a review from a team as a code owner June 18, 2024 02:24
@mjhuff mjhuff requested review from brenthagen and removed request for a team and brenthagen June 18, 2024 02:24
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno, I do think it's nice to have hooks in hooks/ and stuff that's not hooks in utils/. I appreciate that they might be used in different ways, and that the hooks do have standard names, but I think it's just nice and neat to keep them in separate directories also.

@mjhuff
Copy link
Contributor Author

mjhuff commented Jun 18, 2024

Per convo, we'll go with separate directories for now with the intention of breaking the hooks directory into data vs. component-utilizing-hooks later (all the ER hooks are data hooks, currently).

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@mjhuff mjhuff merged commit 70b4174 into edge Jun 18, 2024
25 checks passed
@mjhuff mjhuff deleted the app_er-hooks-to-utils branch June 18, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants