Skip to content
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

Orphans in non showcontent categories #612

Merged
merged 10 commits into from
Jun 14, 2024

Conversation

SmileyChris
Copy link
Contributor

@SmileyChris SmileyChris commented Jun 13, 2024

Description

Currently, orphans (newsfragments without an issue number) don't display correctly in a non "showcontent" category, such as the "misc" category.

These fragments don't have an issue number, but if they were important enough to have a fragment, I still think their content should then be shown.

Checklist

  • Make sure changes are covered by existing or new tests.
  • For at least one Python version, make sure local test run is green.
  • Create a file in src/towncrier/newsfragments/. Describe your
    change and include important information. Your change will be included in the public release notes.
  • Make sure all GitHub Actions checks are green (they are automatically checking all of the above).
  • Ensure documentation is still up-to-date.

@SmileyChris SmileyChris force-pushed the non-showcontent-orphans branch from 11f7606 to 86911a1 Compare June 13, 2024 04:38
@SmileyChris SmileyChris marked this pull request as ready for review June 13, 2024 04:49
@SmileyChris SmileyChris requested a review from a team as a code owner June 13, 2024 04:49
docs/configuration.rst Outdated Show resolved Hide resolved
docs/configuration.rst Outdated Show resolved Hide resolved
@@ -15,7 +15,6 @@
{% for category, val in definitions.items() if category in sections[section] %}
### {{ definitions[category]['name'] }}

{% if definitions[category]['showcontent'] %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should add an 612.removal fragment, informat that when showcontent = False the template will no longer receive the fragment content.

This is a note, just in case someone was missusing this feature with a custom template.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's some confusion here. The existing behaviour is already that fragment content is not sent to the template (https://github.com/twisted/towncrier/pull/612/files#diff-36db088409efe07783d49792a18cec6ae3778ace338bf1cd08daa8a3fce29e93L166). This change just means that orphan fragment contents are sent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. My bad. I misinterpreted the changes in the code.

Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Looks good.

My only major comment is to add a .removal release note informing that the Jinja2 template will no longer receive the fragment content, for showcontent = False categories.

@SmileyChris SmileyChris requested a review from adiroiban June 13, 2024 21:36
SmileyChris and others added 3 commits June 14, 2024 09:59
* Refactor rendering of title via config.title_format

* Add newsfragment

* Config docs

* Fix restructuredtext formatting error
…wisted#608)

* Refactor issue_key function to sort issues in a human-friendly way

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Rename newsfragment

* Small improvement to test to show how text with numeric issues are sorted

* Update src/towncrier/_builder.py docstring grammar

Co-authored-by: Adi Roiban <[email protected]>

* clarify new behaviour in newsfragment

* Add some docstrings/comments to tests

* linelength fix

* Clarify news fragments vs tickets

Co-authored-by: Adi Roiban <[email protected]>

* Consistent use of "issue" rather than "ticket"

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* typo

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Adi Roiban <[email protected]>
Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good. Thanks.

Sorry for the previous review that was mediocre.

@SmileyChris SmileyChris merged commit 09819ac into twisted:trunk Jun 14, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants