Ensure that changelog entries are ordered #613
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexwlchan
reviewed
May 10, 2017
scripts/check-changelog.py
Outdated
| for line in tools.changelog().split('\n'): | ||
| if line.strip() in acceptable_lines: | ||
| pattern = r'\n\d+\.\d+\.\d+ - \d\d\d\d-\d\d-\d\d\n' | ||
| all_versions = list(map(str.strip, re.findall(pattern, tools.changelog()))) |
There was a problem hiding this comment.
Possibly cleaner?
pattern = re.compile(r'\n(?<line>\d+\.\d+\.\d+ - \d{4}-\d{2}-\d{2})\n')
all_versions = [m.group('line') for m in pattern.finditer(tools.changelog())]There was a problem hiding this comment.
I like the date pattern better, but otherwise I find it easier to think about re.findall returning a list of strings.
scripts/check-changelog.py
Outdated
| else: | ||
| print('No line with version and current date (%s) in the changelog. ' | ||
| 'Remember this will be released as soon as you merge to master!' | ||
| % ' or '.join(repr(line) for line in acceptable_lines)) | ||
| sys.exit(1) | ||
|
|
||
| assert all_versions == sorted(all_versions), \ | ||
| 'You edited old release notes and changed the ordering!' |
There was a problem hiding this comment.
Personally, I’d make this an if statement instead:
if all_versions != sorted(all_versions):I reserve assert for "the program has got into an unexpected/should-be-impossible state, abort!" – whereas it's totally possible that one of us screws up the changelog. 😉
There was a problem hiding this comment.
Agreed - I should have done that when I thought a message might be useful!
Zac-HD
force-pushed
the
precise-changelog-check
branch
from
ea027ea to
fd84c2f
Compare
I've had some trouble with the changelog when rebasing or merging, so I thought I'd write some more checks. Now the expected entry must appear first, all version numbers must be unique and in order, and no version numbers may be skipped. See https://xkcd.com/208/
Zac-HD
force-pushed
the
precise-changelog-check
branch
from
c27b104 to
b4badec
Compare
DRMacIver
approved these changes
May 15, 2017
|
Excellent - having worked out all the ways that merging could break the changelog but not testing for them was going to make me nervous 😄 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I just had the changelog for #508 (proposed version 3.9.0) merged to below the changelog for new release 3.8.3, so I thought our check should look at order as well as existence. And uniqueness. And that we didn't skip a valid version number.