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
ENT-3638: Add commit title to changelog by default, if it has ticket … #3104
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3104 +/- ##
=======================================
Coverage 61.98% 61.98%
=======================================
Files 210 210
Lines 46216 46216
=======================================
Hits 28648 28648
Misses 17568 17568 Continue to review full report at Codecov.
|
f9a308c
to
15c17b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. The HACKING.md file should be updated accordingly.
15c17b0
to
47f1059
Compare
HACKING.md has this line at the end:
which looks silly now that I added line about CFE-1234 in the title |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you be able to suppress an entry using Changelog: None
(if that's even desirable)?
Also, do you know that there is a test in the same directory? I have never run it automatically, since the tool changes so infrequently, but might be good to keep it up to date so you don't accidentally break something unrelated.
oh, Kristian, you're right: |
55b2bd5
to
7c3442a
Compare
Ok, 2 months and 1 week later I looked at this code once again and now I realised now to fix it without completely rewriting everything. Also, two new tests added and all tests are passing. Thanks for the hint, @kacf ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to. I cannot say that about the rest of the code, though 😄
JIRA_REGEX = r"(?:Jira:? *)?(?:https?://tracker.mender.io/browse/)?((?:CFE|ENT|INF|ARCHIVE|MEN|QA)-[0-9]+)" | ||
# more strict regexp to find JIRA issues only in the beginning of title | ||
JIRA_TITLE_REGEX = r"(?:CFE|ENT|INF|ARCHIVE|MEN|QA)-[0-9]+" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this regex start with ^
then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not necessary. Later it's used in re.match
function, which does an "ancored match" - i.e. implies ^
at the beginning of regexp. For non-anchored match, Python provides a re.search
function:
lex@flower:~$ python3
Python 3.5.2 (default, Nov 23 2017, 16:37:01)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import re
>>> re.match('a','abba')
<_sre.SRE_Match object; span=(0, 1), match='a'>
>>> re.match('b','abba')
>>> re.match('c','abba')
>>> re.search('b','abba')
<_sre.SRE_Match object; span=(1, 2), match='b'>
>>> re.search('c','abba')
>>>
lex@flower:~$
but maybe you're right and it's better to include it for readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the code is quite cryptic. I think it's better to be explicit.
…number From now on, you don't need to write "Changelog: title" if you prepend title with JIRA ticket number Signed-off-by: Aleksei Shpakovskii <aleksei.shpakovskii@cfengine.com>
Now, instead of `git cat-file -p $SHA` and skipping through non-empty lines in the commit header, we just run `git log --format=%B -n 1 $SHA` and get commit message directly. Previous method could break if commit header contained empty lines - this is a case for signed commits
7c3442a
to
3d28e0e
Compare
…number
From now on, you don't need to write "Changelog: title" if you prepend
title with JIRA ticket number
Signed-off-by: Aleksei Shpakovskii aleksei.shpakovskii@cfengine.com