Require parse arguments #1228
Require parse arguments #1228
Conversation
Require arguments to `augur parse` that are actually required including the list of fields to parse and both output files. Adds a simple functional test to confirm this new behavior. Resolves #1222
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1228 +/- ##
=======================================
Coverage 68.87% 68.87%
=======================================
Files 64 64
Lines 6937 6937
Branches 1693 1693
=======================================
Hits 4778 4778
Misses 1854 1854
Partials 305 305
☔ View full report in Codecov by Sentry. |
| @@ -2,21 +2,20 @@ | |||
|
|
|||
| ## __NEXT__ | |||
|
|
|||
| ### Internal | |||
There was a problem hiding this comment.
@huddlej I had put Internal there on purpose, as CI is not a bug fix. Did you remove it intentionally or during merge conflict? :)
There was a problem hiding this comment.
@corneliusroemer Yeah, sorry about that. I wanted to standardize the file while I was in there (I removed some newlines, too) and all of our previous CI changes have been classified as "Bug fixes" for the sake of noting the next release type. I might be overly compulsive about those things though...
There was a problem hiding this comment.
Not sure how much weight I would put on precedent here, it's not like anyone has build a script to parse our change notes that would break if we add a new type of heading (I hope). Had I been aware I would have argued for the change already earlier. I have a strong opinion, but it's weakly held. Happy to leave things as is if others hold their opinions more strongly on this 🙃
There was a problem hiding this comment.
I'm just seeing this conversation. This codebase has (somewhat recently) adopted a practice of not including CI changes in the changelog to keep it relevant for users.
If we were to include them though, I'd be on board with you @corneliusroemer - it's better to separate CI changes from actual "bug fixes" for a similar motivation of readability for users (and developers).
Description of proposed changes
Require arguments to
augur parsethat are actually required including the list of fields to parse and both output files.Related issue(s)
Resolves #1222
Testing
Checklist