add an option to check staged files #677
Conversation
|
FYI: @adiroiban |
There was a problem hiding this comment.
Hi @krpatter-intc, thank you for the contribution!
Overall this looks good, but I had some (admittedly fairly pedantic) feedback to address before we can merge.
Thanks again!
src/towncrier/test/test_check.py
Outdated
| There must be uncommitted changes otherwise git will complain: | ||
| "nothing to commit, working tree clean" | ||
| """ | ||
| call(["git", "add", "."]) |
There was a problem hiding this comment.
Shouldn't this check the exit status?
I suppose the same applies to all the other helper functions here. You needn't fix those, but it would be appreciated.
There was a problem hiding this comment.
Ok, switched all call commands to check_call so an exception is thrown if a call fails
src/towncrier/test/test_check.py
Outdated
| @@ -204,6 +213,44 @@ def test_fragment_exists_but_not_in_check(self): | |||
| (result.output, str(fragment_path)), | |||
| ) | |||
|
|
|||
| def test_fragment_exists_and_staged(self): | |||
| """A fragment that exists but is marked as check=False is ignored by the check.""" | |||
There was a problem hiding this comment.
Please update this test docstring to mention staging.
There was a problem hiding this comment.
fixed in 363532a
src/towncrier/check.py
Outdated
| "staged", | ||
| is_flag=True, | ||
| default=False, | ||
| metavar="STAGED", |
There was a problem hiding this comment.
Does metavar do anything when it's a flag?
There was a problem hiding this comment.
good point. removed in: de88e69
src/towncrier/test/test_check.py
Outdated
| "[[tool.towncrier.type]]\n" | ||
| 'directory = "sut"\n' | ||
| 'name = "System Under Test"\n' | ||
| "showcontent = true\n" | ||
| "check=false\n", |
There was a problem hiding this comment.
I think you could shorten this test by omitting this check=False stuff (and staging the .sut file below).
There was a problem hiding this comment.
fixed in 363532a
There was a problem hiding this comment.
Thanks for the quick turn-around @krpatter-intc. This looks great to me!
Description
Implements #676
Adds new
--stagedparameter to the check command so that staged files are included as part of the check.Checklist
src/towncrier/newsfragments/. Briefly describe yourchanges, with information useful to end users. Your change will be included in the public release notes.
EnsureN/Adocs/tutorial.rstis still up-to-date.docs/cli.rstreflects those changes.If you add new configuration options (or change the meaning of existing ones), make sureN/Adocs/configuration.rstreflects those changes.