-
Notifications
You must be signed in to change notification settings - Fork 122
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
Section aware create #603
Section aware create #603
Conversation
Thanks for the PR. As long as you are happy with the behaviour, we can merge it. I am tring to understand what "sections" is in the context of towncrier. I am reading something about it here https://towncrier.readthedocs.io/en/latest/configuration.html#sections Especially the part from I think that it needs better example. For example
Which Are people using this behaviour ? To me, this all seems very confusing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for the PR. It looks very good.
I left an initial round of feedback.
I have never used the create
command .
Also, for my privarte project, I am not using the "section" functionality.
I know that "sections" are used in twisted/twisted
without a path ... and that's all my experience with it.
In twisted/twisted
there is a single section without a path.
The major concert is the backward incompatible change for the create
CLI
I don't know why you would want multiple sections without a path.... so maybe this is not a big deal, and it's ok to have the current behaviour.
I would still document this as .removal
.
@@ -53,6 +55,35 @@ def parse_newfragment_basename( | |||
return invalid | |||
|
|||
|
|||
class FragmentsPath: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this expected to be public API?
I can do something like this: from towncrier.create import FragmentsPath
…uldn't have worked previously anyway
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks all good. I saw the auto-merge, so I hope this will be merged.
Description
towncrier create
should work better with sections if defined in the configuration file.This adds a new
towncrier create --section <section name>
option, or if you're using the new interactive create then you'll be asked for the section if there are multiple ones defined in your config:If you use sections and none have an empty path, you must now specify the section when creating a new news fragment (if one does have an empty path, that section will be used by default).
This PR also deduplicates the code that determines the base fragment path which was done in three places!
Checklist
src/towncrier/newsfragments/
. Describe yourchange and include important information. Your change will be included in the public release notes.
docs/tutorial.rst
is still up-to-date.docs/cli.rst
reflects those changes.docs/configuration.rst
reflects those changes.