Check if any news fragments have invalid filenames #622

MetRonnie · 2024-07-10T15:59:57Z

Description

Closes #619
Fixes #426

towncrier check will now fail if any news fragments have invalid filenames.

Added a new configuration option called build_ignore_filenames that allows you to specify a list of filenames that should be ignored. If this is set, towncrier build will also fail if any filenames are invalid, except for those in the list.

Added a --strict option to towncrier build to enable this behaviour for when build_ignore_filenames has not been configured.

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 docs/tutorial.rst is still up-to-date.
If you add new CLI arguments (or change the meaning of existing ones), make sure docs/cli.rst reflects those changes.
If you add new configuration options (or change the meaning of existing ones), make sure docs/configuration.rst reflects those changes. (Not applicable)

src/towncrier/_builder.py

src/towncrier/build.py

adiroiban

Thanks for working on this.

This needs documentation and tests to be merged.

While reviewing this, my question is.

Why do we ever want to allow towncrier build or towncrier check to succeed if there are in invalid fragments.

I understand that you might want to have other files inside the newsfragment folder (for example the template and a README) but other than that, I think towncrier should fail

Cheers

MetRonnie · 2024-07-12T11:13:52Z

I think all feedback has been addressed now. Let me know if there is anything else.

Maybe a warning if towncrier build encounters invalid filenames but build_ignore_filenames has not been configured? Do you envisage deprecating the --strict option and making the fail behaviour the default in a future version of Towncrier?

adiroiban

Many thanks for the changes.

They look good.

As a general rule, I think that is best to start any new functionality by writing the documentation.

In this way, we can agree on how the feature should behave, before writing more code.

I think that we can have this implemented without the --strict option.

In terms for testing I am expecting 3 main test scenarios:

build_ignore_filenames not set
build_ignore_filenames is set to empty list
build_ignore_filenames is set with some filenames

Thanks again for working on this!

docs/cli.rst

docs/configuration.rst

docs/tutorial.rst

src/towncrier/_builder.py

src/towncrier/test/test_build.py

src/towncrier/test/test_check.py

docs/configuration.rst

adiroiban · 2024-07-15T10:33:27Z

I forgot to leave this feedback

Maybe a warning if towncrier build encounters invalid filenames but build_ignore_filenames has not been configured?

That would be nice. We can also have this in a separate PR.
This should not be a blocker.

Do you envisage deprecating the --strict option and making the fail behaviour the default in a future version of Towncrier?

I hope that we don't have to add the --strict option... so that we don't have to deprecate it.

It would be nice to have a warinig raised when build_ignore_filenames is not configured and make a release that raised the warning.

In a followup release, we can then change the default value of build_ignore_filenames so thta it will alwasy do a strict check by default... while still allowing users to configure it with build_ignore_filenames = None`

adiroiban · 2024-07-15T10:35:53Z

I have updated the PR description to note that it will also fix #426 ... the #619 might be a duplicate

- Change `build_ignore_filenames` to `ignore` in config - Remove `--strict` option from `towncrier build`

adiroiban

Many thanks for the changes.

They look good.

I have to still review the automated tests... but the other part looks good.

src/towncrier/_builder.py

adiroiban · 2024-07-15T11:27:50Z

src/towncrier/check.py

@@ -101,12 +101,14 @@ def __main(
        click.echo(f"{n}. {change}")
    click.echo("----")

+    # This will fail if any fragment files have an invalid name:


We can keep it. No problem.

The critical part is to make sure that we have automated tests that will fail is anyone would move this code :)

docs/tutorial.rst

adiroiban · 2024-07-15T15:22:01Z

src/towncrier/newsfragments/622.feature.rst

@@ -0,0 +1,3 @@
+``towncrier check`` will now fail if any news fragments have invalid filenames.


Not sure how this will ended up in the rendered version.

We should have 2 fragments here.
One for towncrier check

and another one for the ignore configuration option.

But maybe the towncrier check part can be left out.

If build fails, it should be expect for check to also fail.

I feel like the addition of the config option and the new fail behaviour are intertwined, so I think it makes sense to have them in the same fragment. At least I think the change in behaviour of towncrier check should be clearly signposted.

I could rewrite this to be clearer, how about:

towncrier check will now fail if any news fragments have invalid filenames. You can specify filenames to ignore in the news fragments directory with the new ignore configuration option. towncrier build will not fail on invalid filenames unless ignore is set (can be an empty list).

As you think it's best

we can leave it as it is. no problem.

…tically ignored filenames

adiroiban

Looks good. Many thanks for the changes.

adiroiban · 2024-07-25T16:10:48Z

src/towncrier/check.py

@@ -101,12 +101,14 @@ def __main(
        click.echo(f"{n}. {change}")
    click.echo("----")

+    # This will fail if any fragment files have an invalid name:


Thanks. I didn't had time to check the test coverage... but if say that you have checked it, we can have this merged :)

no problem

adiroiban · 2024-07-25T16:11:21Z

src/towncrier/newsfragments/622.feature.rst

@@ -0,0 +1,3 @@
+``towncrier check`` will now fail if any news fragments have invalid filenames.


As you think it's best

we can leave it as it is. no problem.

adiroiban · 2024-07-29T14:49:03Z

Thanks for the update. I have enabled auto-merge.

I hope this will be merged soon.

I plan to do a new release soon... I hope #627 will also be merge soon and then we can have a release.

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

Reference: twisted/towncrier#622

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

Merge after the inclusion of twisted/towncrier#622 in the next release of towncrier.

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

MetRonnie commented Jul 10, 2024

View reviewed changes

src/towncrier/_builder.py Outdated Show resolved Hide resolved

MetRonnie force-pushed the check-fragment-names branch from a4f1740 to 2ec1c4a Compare July 10, 2024 16:04

This was referenced Jul 10, 2024

Check for invalid filenames in changes.d/ #619

Closed

check towncrier message format in lint ci cylc/cylc-flow#6216

Closed

adiroiban reviewed Jul 10, 2024

View reviewed changes

src/towncrier/_builder.py Outdated Show resolved Hide resolved

adiroiban reviewed Jul 10, 2024

View reviewed changes

src/towncrier/_builder.py Outdated Show resolved Hide resolved

adiroiban reviewed Jul 10, 2024

View reviewed changes

src/towncrier/_builder.py Outdated Show resolved Hide resolved

adiroiban reviewed Jul 10, 2024

View reviewed changes

src/towncrier/build.py Outdated Show resolved Hide resolved

adiroiban requested changes Jul 10, 2024

View reviewed changes

MetRonnie force-pushed the check-fragment-names branch from 2ec1c4a to b85391c Compare July 12, 2024 11:06

Check for invalid fragment names

0993534

MetRonnie force-pushed the check-fragment-names branch from b85391c to 0993534 Compare July 12, 2024 11:09

MetRonnie changed the title ~~Add --check-names option to towncrier build~~ Check if any news fragments have invalid filenames Jul 12, 2024

MetRonnie marked this pull request as ready for review July 12, 2024 11:11

MetRonnie requested a review from a team as a code owner July 12, 2024 11:11

adiroiban requested changes Jul 13, 2024

View reviewed changes

adiroiban mentioned this pull request Jul 13, 2024

Release time? #616

Closed

adiroiban mentioned this pull request Jul 15, 2024

Multiple fragment types for same fragment (e.g. .bug and .bugfix) #426

Closed

MetRonnie added 2 commits July 15, 2024 12:17

Refactor in response to review

3c93f13

- Change `build_ignore_filenames` to `ignore` in config - Remove `--strict` option from `towncrier build`

Merge 'upstream/trunk' into check-fragment-names

618e325

adiroiban reviewed Jul 15, 2024

View reviewed changes

MetRonnie added 2 commits July 15, 2024 18:20

Tidy docs

935a831

Make config.ignore filename list case-insensitive & add more automa…

26c60ff

…tically ignored filenames

MetRonnie force-pushed the check-fragment-names branch from 5bba4eb to 26c60ff Compare July 15, 2024 17:47

MetRonnie added 2 commits July 16, 2024 15:43

Improve test

060c5d3

Merge branch 'trunk' into check-fragment-names

2ce2b27

adiroiban approved these changes Jul 25, 2024

View reviewed changes

Improve test coverage

9b68503

adiroiban enabled auto-merge (squash) July 29, 2024 14:48

adiroiban merged commit 4b58be5 into twisted:trunk Jul 29, 2024
16 checks passed

MetRonnie deleted the check-fragment-names branch July 29, 2024 15:18

MetRonnie mentioned this pull request Jul 30, 2024

Upgrade towncrier cylc/cylc-flow#6269

Merged

8 tasks

aviramha mentioned this pull request Jul 31, 2024

Regression/breakage on 24.7.0 - check for invalid files #632

Closed

johannaengland added a commit to johannaengland/zino that referenced this pull request Jul 31, 2024

Add gitkeep file to ignored towncrier files

5c5cd38

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

johannaengland mentioned this pull request Jul 31, 2024

Add gitkeep file to ignored towncrier files Uninett/zino#334

Merged

6 tasks

johannaengland added a commit to johannaengland/Argus that referenced this pull request Jul 31, 2024

Add gitkeep file to ignored towncrier files

63c029e

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

johannaengland mentioned this pull request Jul 31, 2024

Add gitkeep file to ignored towncrier files Uninett/Argus#853

Merged

johannaengland added a commit to Uninett/Argus-frontend that referenced this pull request Jul 31, 2024

Release 24.7.0 made this necessary by scanning for invalid filenames

96cdb37

Reference: twisted/towncrier#622

johannaengland mentioned this pull request Jul 31, 2024

Add gitkeep file to ignored towncrier files Uninett/Argus-frontend#572

Merged

johannaengland added a commit to johannaengland/nav that referenced this pull request Jul 31, 2024

Add gitkeep file to ignored towncrier files

f63fe00

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

johannaengland added a commit to Uninett/Argus-frontend that referenced this pull request Jul 31, 2024

Add gitkeep file to ignored towncrier files

8e27a92

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

johannaengland mentioned this pull request Jul 31, 2024

Add gitkeep file to ignored towncrier files Uninett/nav#2942

Merged

johannaengland added a commit to Uninett/zino that referenced this pull request Aug 1, 2024

Add gitkeep file to ignored towncrier files

22ce05a

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

johannaengland added a commit to Uninett/nav that referenced this pull request Aug 1, 2024

Add gitkeep file to ignored towncrier files

98755bb

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

johannaengland added a commit to Uninett/Argus-frontend that referenced this pull request Aug 1, 2024

Add gitkeep file to ignored towncrier files

ef61bed

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

johannaengland added a commit to Uninett/Argus that referenced this pull request Aug 1, 2024

Add gitkeep file to ignored towncrier files

485db50

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

eigenbrot mentioned this pull request Aug 1, 2024

Allow wildcards in "ignore" configuration option #642

Closed

johannaengland added a commit to johannaengland/zino that referenced this pull request Aug 2, 2024

Add gitkeep file to ignored towncrier files

d840dcc

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

This was referenced Aug 2, 2024

Add towncrier files to ignore or rename .keep plone/volto#6225

Closed

Add .gitkeep as one of default valid filenames #643

Closed

Add .gitkeep as an ignored filename #647

Merged

stevepiercy added a commit to plone/volto that referenced this pull request Aug 4, 2024

Revert rename of .gitkeep.

2357f75

Merge after the inclusion of twisted/towncrier#622 in the next release of towncrier.

stevepiercy mentioned this pull request Aug 4, 2024

Revert rename of .gitkeep for consistency plone/volto#6232

Merged

jorund1 pushed a commit to jorund1/nav that referenced this pull request Aug 8, 2024

Add gitkeep file to ignored towncrier files

50b255d

Release 24.7.0 made this necessary by scanning for invalid filenames Reference: twisted/towncrier#622

This was referenced Aug 9, 2024

Towncrier: enable checking of fragment names cylc/cylc-ui#1889

Merged

Towncrier: check fragment names cylc/cylc-uiserver#622

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check if any news fragments have invalid filenames #622

Check if any news fragments have invalid filenames #622

MetRonnie commented Jul 10, 2024 •

edited by adiroiban

Loading

adiroiban left a comment

MetRonnie commented Jul 12, 2024 •

edited

Loading

adiroiban left a comment

adiroiban commented Jul 15, 2024 •

edited

Loading

adiroiban commented Jul 15, 2024

adiroiban left a comment

adiroiban Jul 15, 2024

adiroiban Jul 15, 2024

MetRonnie Jul 15, 2024 •

edited

Loading

adiroiban Jul 25, 2024

adiroiban left a comment

adiroiban Jul 25, 2024

adiroiban Jul 25, 2024

adiroiban commented Jul 29, 2024

		@@ -0,0 +1,3 @@
		``towncrier check`` will now fail if any news fragments have invalid filenames.

Check if any news fragments have invalid filenames #622

Check if any news fragments have invalid filenames #622

Conversation

MetRonnie commented Jul 10, 2024 • edited by adiroiban Loading

Description

Checklist

adiroiban left a comment

Choose a reason for hiding this comment

MetRonnie commented Jul 12, 2024 • edited Loading

adiroiban left a comment

Choose a reason for hiding this comment

adiroiban commented Jul 15, 2024 • edited Loading

adiroiban commented Jul 15, 2024

adiroiban left a comment

Choose a reason for hiding this comment

adiroiban Jul 15, 2024

Choose a reason for hiding this comment

adiroiban Jul 15, 2024

Choose a reason for hiding this comment

MetRonnie Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

adiroiban Jul 25, 2024

Choose a reason for hiding this comment

adiroiban left a comment

Choose a reason for hiding this comment

adiroiban Jul 25, 2024

Choose a reason for hiding this comment

adiroiban Jul 25, 2024

Choose a reason for hiding this comment

adiroiban commented Jul 29, 2024

MetRonnie commented Jul 10, 2024 •

edited by adiroiban

Loading

MetRonnie commented Jul 12, 2024 •

edited

Loading

adiroiban commented Jul 15, 2024 •

edited

Loading

MetRonnie Jul 15, 2024 •

edited

Loading