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

CheckJira should be able to disregard some commits by their message contents #58

Open
gnustavo opened this issue Jan 17, 2019 · 7 comments · May be fixed by #77
Open

CheckJira should be able to disregard some commits by their message contents #58

gnustavo opened this issue Jan 17, 2019 · 7 comments · May be fixed by #77
Assignees

Comments

@gnustavo
Copy link
Owner

The CheckJira plugin looks for Jira keys in the commit messages and is usually configured to abort if it can't find any.

However, when it processes commits generated automatically (such as merge, squash, and revert commits) the messages are generated automatically and may not satisfy all CheckJira constraints.

There are situations in which these commits are generated at the git server (such as when you interate a pull request in Bitbucket server) and the user don't have a chance to amend the commit before it's fed to the hook.

So, it's useful to be able to detect and disregard such commits.

The idea is to implement a new directive called githooks.checkjira.skip-message which would accept one or more regexes. If any one of them maches the commit message, the commit should be skipped from consideration by the hook.

@gnustavo gnustavo self-assigned this Jan 17, 2019
@boudekerk
Copy link
Contributor

I think for most cases this can already be achieved by combining the CheckJira and CheckLog plugins:

githooks.checkjira.matchkey=\b[A-Z][A-Z]+\d?-\d+\b
githooks.checkjira.require=false
githooks.checklog.title-match=(\b[A-Z][A-Z]+\d?-\d+\b)|<CUSTOM COMMIT MESSAGE>

@gnustavo
Copy link
Owner Author

Clever, @boudekerk !

The error message would be very opaque to the user, though. I think this specific case deserves a specific solution in CheckJira.

Or perhaps a more general solution common to all plugins... Something like githooks.dont-check-commit-matching-message.

But your solution can solve the problem right now. Thanks!

@boudekerk
Copy link
Contributor

We are actually running into the issue where people seeing it the first time aren't clear about the issue. So if the specific solution in CheckJira would be considered, that might be nice.

The general one I think would benefit from a second property specifying which plugins. I can imagine that you wouldn't want there to be a message to skip every check. For example ACL, but which ones might be different for everyone.

@gnustavo
Copy link
Owner Author

Hi @boudekerk ,

Can you try the new CheckJira's skip-logs directive I implemented in the commit bf81b1a to see if it works for you?

@boudekerk
Copy link
Contributor

boudekerk commented Oct 30, 2023

I just tested it, and that would work great for us. Thanks!

But I'm not sure skip-logs should have a default like that? It's good to have a sensible suggestion for a value in the documentation, but adding it by default. does break backwards compatibility.

And I probably wouldn't have the revert in there, that might very well warrant a Jira ticket with an explanation.

P.S. Any chance you could have a look at #77 ?

gnustavo added a commit that referenced this issue Nov 2, 2023
@gnustavo
Copy link
Owner Author

gnustavo commented Nov 2, 2023

@boudekerk , I've rewritten that commit to remove the default and to mention it in the documentation.

I created PR #78. Can you comment on it?

Regarding #77 , I'm thinking about it. I'm not sure what's the best way to support more than one Jira server. We have two at work, but so far only one of them is used for software development, so that we've never found the need to mention the other one in our commits. But I can see that it would be useful. I'll comment on it when I have something sensible to say.

gnustavo added a commit that referenced this issue Nov 3, 2023
@boudekerk
Copy link
Contributor

#78 Looks good to me.

Regarding my PR #77. Are you doubtful about my chosen solution, or the way I implemented it? I know I'm not a developer, so it might not be the best and/or nicest code.

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

Successfully merging a pull request may close this issue.

2 participants