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

fix(hooks): make the output more uniform and delineated #2343

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshtrichards
Copy link
Member

Previously:

  • Some essentially similar hook folder states would result in different output (e.g. nonexistent versus empty folders)
  • The completion of a given stage wasn't 100% delineated

With this PR, the broader hook handling output is more neutral/consistent while retaining detailed hints where appropriate about the state of things along the way.

Specifically:

  • Always indicate when we're searching for scripts for a given stage
  • Skip early if a given stage's folder is empty too
  • Always indicate when we're skipping (executing nothing for) a given stage + why
  • Always delineate when we've completely finished executing all found scripts for given stage
  • Add the term we use for this feature in our docs to the output ("hook")
  • Minor language tweaks

Tested with:

  • Empty hook folders
  • Nonexistent hook folders
  • Scripts lacking exec permission in hook folder
  • Non-*.sh files in hook folders
  • Valid executable scripts in hook folder

- Use "Searching for scripts [...] located in [...]" consistently (i.e. for each hook_folder_path instead of only for some / under some conditions)
- Skip early if a given hook folder is empty too (not just nonexistent)
- Add feature name (hooks) to all messaging for clarity

Signed-off-by: Josh <[email protected]>
@joshtrichards joshtrichards changed the title fix(hooks): make the output more uniform an delineated fix(hooks): make the output more uniform and delineated Nov 26, 2024
@joshtrichards
Copy link
Member Author

Cc: @dvaerum, @d0niek FYI: Just some (minor) hook related tidying

@dvaerum
Copy link
Contributor

dvaerum commented Nov 26, 2024

Generally it looks fine to me, but you will probably be told to fix your git history because your 2nd commit makes corrects things in your 1st commit.

I would, for readability, make the 1 commit (the 1st) focus on the logic and have a 2nd commit which fixed the echo messages there you want them improved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants