Changelog: Add missing links #4256
Conversation
This is now part of mixxxdj/mixxx#4256.
|
@daschuer Anything else to do here? |
|
Merge? This just adds missing issue links to the changelog and ensures that missing links are detected beforehand in the future. |
tools/changelog.py
Outdated
| args.file.seek(0) | ||
| args.file.write(fixed_changelog) | ||
| args.file.truncate() |
There was a problem hiding this comment.
will this cause issues when the changelog is being read from stdin? We should handle this case and just print to stdout in that case.
There was a problem hiding this comment.
Good point, I replaced argparse.FileType with pathlib.Path, so passing - just tries to open a file with that name rather than passing a std stream.
There was a problem hiding this comment.
I guess working around a bug by just removing the feature is a solution...
Since this is not intended to be a super slick tool anyways, I can look past the hacky-ness.
tools/changelog.py
Outdated
| fixed_changelog = add_missing_links(changelog) | ||
| if changelog != fixed_changelog: | ||
| args.file.seek(0) | ||
| args.file.write(fixed_changelog) |
There was a problem hiding this comment.
Why do we open the file with r+ but then be able to write to it here? Seems like it could cause undefined behavior?
There was a problem hiding this comment.
The python docs confuse me here...
https://docs.python.org/3/library/functions.html#open
| Character | Meaning |
|---|---|
| 'r' | open for reading (default) |
| 'w' | open for writing, truncating the file first |
| 'x' | open for exclusive creation, failing if the file already exists |
| 'a' | open for writing, appending to the end of the file if it exists |
| 'b' | binary mode |
| 't' | text mode (default) |
| '+' | open for updating (reading and writing) |
The default mode is 'r' (open for reading text, synonym of 'rt'). Modes 'w+' and 'w+b' open and truncate the file. Modes 'r+' and 'r+b' open the file with no truncation.
There was a problem hiding this comment.
Clarified the situation by just opening the file separately for reading and writing.
There was a problem hiding this comment.
That wouldn't have been necessary, but its much clearer now anways. Thanks
By using `pathlib.Path` instead of `argparse.FileType`, we can restrict the files to actual paths (not stdin/stdout/stderr) and also get rid of the ugly seeking/truncating and manual closing of the file.
|
Sorry forgot to press merge... |
As requested by @daschuer here: mixxxdj/manual#422 (comment)