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

handle changelog entries with towncrier #1826

Merged
merged 6 commits into from
Aug 19, 2024

Conversation

zacharyburnett
Copy link
Member

@zacharyburnett zacharyburnett commented Aug 16, 2024

Description

towncrier expects "news fragment" files (text files in the changes/ directory with filenames in the format <PR#>.<changetype>.rst, i.e. for this PR it would be changes/1826.misc.rst). See docs at https://towncrier.readthedocs.io/en/latest/tutorial.html#creating-news-fragments

when ready to make a release, run towncrier build to ingest the news fragments and generate a changelog section in CHANGES.rst with all the new change log entries for that release (this clears the changes/ directory). This step should either be done before making a release, or could probably be added to a GitHub workflow triggered on release (to insert a commit and remake the tag).

After merging this PR the Release Process wiki page will need to be updated to include a step to run towncrier and add to CHANGES.rst.

Currently this PR uses the default towncrier change types:

  • feature: Signifying a new feature.
  • bugfix: Signifying a bug fix.
  • doc: Signifying a documentation improvement.
  • removal: Signifying a deprecation or removal of public API.
  • misc: An issue has been closed, but it is not of interest to users.

However, you can also define custom types; I think it might be easier for users to use the above sections in the changelog instead of what we currently use (more or less one section per-module). I can add those sections back though, if that's better

Checklist:

  • pre-commit checks ran successfully
  • tests ran successfully
  • for a public change, a changelog entry was added
  • for a public change, documentation was updated
  • for any new features, unit tests were added

@zacharyburnett zacharyburnett changed the title add towncrier configuration use towncrier to handle changelog entries Aug 16, 2024
@zacharyburnett zacharyburnett changed the title use towncrier to handle changelog entries handle changelog entries with towncrier Aug 16, 2024
@zacharyburnett zacharyburnett marked this pull request as ready for review August 16, 2024 16:22
@zacharyburnett zacharyburnett requested a review from a team as a code owner August 16, 2024 16:22
@braingram
Copy link
Contributor

How could we update our changelog check to use the new format? I see the changelog-bot configuration in this PR do we have to enable that bot? If so, is there a way that doesn't require another bot (like an action that uses towncrier check?

Also, for the "running changelog" I'm thinking one option might be to add a towncrier build --draft and redirect it's output to the action summary for the nightly cron job. Are there other options that might be useful here to be able to get a (reasonably up-to-date) running changelog for what will go into the next build?

@zacharyburnett
Copy link
Member Author

zacharyburnett commented Aug 19, 2024

How could we update our changelog check to use the new format? I see the changelog-bot configuration in this PR do we have to enable that bot? If so, is there a way that doesn't require another bot (like an action that uses towncrier check?

towncrier check seems to do this, by doing a very simple check for if a new file has been added by the current branch in changes/ (this current branch passes that check because of changes/.gitkeep)

Also, for the "running changelog" I'm thinking one option might be to add a towncrier build --draft and redirect it's output to the action summary for the nightly cron job. Are there other options that might be useful here to be able to get a (reasonably up-to-date) running changelog for what will go into the next build?

I think this might be unnecessary, as anyone can run towncrier build --draft locally if they need it (it would be just as much effort as checking the run summaries)

@zacharyburnett zacharyburnett marked this pull request as draft August 19, 2024 15:17
@zacharyburnett zacharyburnett force-pushed the changelog/towncrier branch 2 times, most recently from 26cc7b6 to bfedcfa Compare August 19, 2024 15:29
@braingram
Copy link
Contributor

Is there a way to tell towncrier to check for a fragment for a particular PR number? Let's say we open a PR 1900 with some feature. Is there a way to check that a 1900.<something>.rst exists and not say 19000.<something>.rst?

@zacharyburnett
Copy link
Member Author

Is there a way to tell towncrier to check for a fragment for a particular PR number? Let's say we open a PR 1900 with some feature. Is there a way to check that a 1900.<something>.rst exists and not say 19000.<something>.rst?

the docs aren't clear: https://towncrier.readthedocs.io/en/stable/cli.html#towncrier-check

I'll test with another test branch

@zacharyburnett
Copy link
Member Author

zacharyburnett commented Aug 19, 2024

I don't see how it would have a way to know the PR number; I think that's why the changelog bot is used.

❯ towncrier check --compare-with changelog/towncrier
Looking at these files:
----
1. /Users/zburnett/projects/asdf/README.rst
----
No new newsfragments found on this branch.
❯ touch changes/2122.misc.rst
❯ towncrier check --compare-with changelog/towncrier
Looking at these files:
----
1. /Users/zburnett/projects/asdf/README.rst
2. /Users/zburnett/projects/asdf/changes/2122.misc.rst
----
Found:
1. /Users/zburnett/projects/asdf/changes/2122.misc.rst

@braingram
Copy link
Contributor

I don't see how it would have a way to know the PR number; I think that's why the changelog bot is used.

Thanks for checking. I guess we could re-introduce the regex and update it to check either the output of towncrier build --draft or for a file <PR number>.<something>.rst. It seems like an important feature to keep and I'm hesitant to enable another bot.

Also, do you have a suggestion for our docs? They list the changelog contents:

.. include:: ../../CHANGES.rst

and I can see a benefit to generating a "draft" changelog for PR docs builds (to make sure the added changelog doesn't cause a sphinx error on release).

@zacharyburnett
Copy link
Member Author

zacharyburnett commented Aug 19, 2024

we could add towncrier build to the changelog checker after checkout, and also the readthedocs run

@zacharyburnett zacharyburnett force-pushed the changelog/towncrier branch 2 times, most recently from 99de7eb to 4dfee81 Compare August 19, 2024 16:30
@zacharyburnett
Copy link
Member Author

@zacharyburnett zacharyburnett marked this pull request as ready for review August 19, 2024 17:25
@zacharyburnett zacharyburnett force-pushed the changelog/towncrier branch 2 times, most recently from 9a078f7 to 4affaae Compare August 19, 2024 17:49
@zacharyburnett zacharyburnett marked this pull request as draft August 19, 2024 17:51
@zacharyburnett zacharyburnett force-pushed the changelog/towncrier branch 3 times, most recently from f21fdfb to 79d9232 Compare August 19, 2024 18:14
@zacharyburnett zacharyburnett marked this pull request as ready for review August 19, 2024 18:18
changelog:
name: Confirm changelog entry
check:
if: ${{ !contains(github.event.pull_request.labels.*.name, 'no-changelog-entry-needed') }}
Copy link
Contributor

Choose a reason for hiding this comment

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

One last question related to these changes and the branch protection.

Our changelog check is listed as one of the branch protection rules (I am in the process of updating these so it might not show as required at the moment). I see this skip is at the job level now instead of the step and that the job shows as skipped. Previously when the no-changelog-entry-needed label was added the job still ran but the check was skipped (see this run) which I believe allowed the branch protection to pass. Will it still pass if the job is skipped (and not just the check)?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes indeed, as evidenced by this PR

Copy link
Contributor

@braingram braingram left a comment

Choose a reason for hiding this comment

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

Thanks for putting this all together!

@braingram braingram merged commit 229269a into asdf-format:main Aug 19, 2024
32 checks passed
@zacharyburnett zacharyburnett deleted the changelog/towncrier branch August 19, 2024 20:01
@braingram
Copy link
Contributor

I think the regex isn't quite right:
https://github.com/asdf-format/asdf/actions/runs/10460294667/job/28966161889?pr=1825
and the docs are failing to build with a fragment (it looks to be waiting for user input):
https://readthedocs.org/projects/asdf/builds/25358714/

@zacharyburnett
Copy link
Member Author

zacharyburnett commented Aug 19, 2024

I think the regex isn't quite right: https://github.com/asdf-format/asdf/actions/runs/10460294667/job/28966161889?pr=1825

fixed in #1828

and the docs are failing to build with a fragment (it looks to be waiting for user input): https://readthedocs.org/projects/asdf/builds/25358714/

I'll take a look fixed in #1829

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants