-
Notifications
You must be signed in to change notification settings - Fork 127
Fix incorrect handling of orphan fragment names consists of only digits #588
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
Changes from 4 commits
26b1d04
d9b4139
5d732ca
6bd47e4
d941865
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Orphan news fragments, fragments not associated with an issue, consisting of only digits (e.g. '+12345678.feature') now retain their leading marker character. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,3 +125,10 @@ def test_orphan_with_dotted_number(self): | |
| parse_newfragment_basename("+orphan_12.3.feature", ["feature"]), | ||
| ("+orphan_12.3", "feature", 0), | ||
| ) | ||
|
|
||
| def test_orphan_all_digits(self): | ||
| """Orphaned snippets can consist of only digits.""" | ||
| self.assertEqual( | ||
| parse_newfragment_basename("+123.feature", ["feature"]), | ||
| ("+123", "feature", 0), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add another non-orphan test with an identifier looking like a commit sha? In the templates I've started using recently, I try to output PRs/issues, commits and arbitrary identifiers separately: https://github.com/cherrypy/cheroot/blob/3591a1c/docs/changelog-fragments.d/.towncrier-template.rst.j2#L59-L63. It'd be nice if that remained functional. Hence an explicit test...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd say that should be a separate PR -- that's an interesting functional test but not really tied to the changes being made here.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason I brought it up is that I wasn't sure if this PR influences it. So it's kinda related as a possible regression. But I understand that it may be non-atomic and separate otherwise.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for your feedback. This test is named I also think that using a git commit ref is also a valid test... but it should be a separate test. |
||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.