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

Git pre-commit hook relies on project.prefix which is not setup by default #4258

Closed
ba66e77 opened this issue Oct 12, 2020 · 2 comments · Fixed by #4265
Closed

Git pre-commit hook relies on project.prefix which is not setup by default #4258

ba66e77 opened this issue Oct 12, 2020 · 2 comments · Fixed by #4265
Labels
Bug Something isn't working

Comments

@ba66e77
Copy link
Contributor

ba66e77 commented Oct 12, 2020

% blt --version
BLT 12.3.0

Describe the bug
After running blt setup, the pre-commit git hook is added and fires when git commit is run. It relies on a project.prefix setting in blt.yml which does not exist by default.

$ git commit -m "commit"
Executing .git/hooks/pre-commit...
Untracked files are present, this may impact the validity of pre-commit checks.
> validate:twig:lint:files
Linting twig files...
All Twig files contain valid syntax.
> validate:yaml:lint:files
Linting YAML files...
Your local code has passed git pre-commit validation.
Executing .git/hooks/commit-msg...
Validating commit message syntax...
[error]  Invalid commit message! 
Commit messages must conform to the regex /(^${project.prefix}-[0-9]+(: )[^ ].{15,}\.)|(Merge branch (.)+)/

To Reproduce
Steps to reproduce the behavior, ideally starting from a fresh install of BLT:

  1. Run acli new and select the acquia/drupal-recommended-project option
  2. Run blt setup
  3. `Git add . ; git commit -m "meh"
  4. Observe error message above

** Expected Behavior **
BLT either sets a project prefix or ignores the prefix and ticket number portion of the commit message check.

@ba66e77 ba66e77 added the Bug Something isn't working label Oct 12, 2020
@danepowell
Copy link
Contributor

danepowell commented Oct 19, 2020

I don't think the root cause here is exactly that the project prefix isn't set, since you'd get the same error if it was set (given that your commit message is "meh"). But I take your point, this isn't a great user experience.

I think we have two options:

  • Set a default project prefix (in BLT 11, it was just "BLT"). We already link to a message about how to change / disable this check in the error output. Every user will have to change this out of the box, which doesn't seem great.
  • Disable commit message validation by default. A bit more user-friendly, but people aren't likely to discover the feature on their own.

@ba66e77 any preferences?

@ba66e77
Copy link
Contributor Author

ba66e77 commented Oct 19, 2020

The ${project.prefix} in the error message would be replaced with the project prefix if one was set.

My preference would be to just set a default prefix. I think the validation is very useful and, like you say, people aren't likely to find it on their own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants