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

Remove unused news file GHA workflow and bot config #13107

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

ichard26
Copy link
Member

@ichard26 ichard26 commented Dec 7, 2024

The PSF Chronographer essentially replaces the news file check GHA workflow. It likes to send a bunch of redundant failure notifications (like when closing stale PRs).

I guess, towncrier does also check that the syntax of a news fragment is correct (although I haven't confirmed this). We could probably throw that in the docs GHA job.

@ichard26 ichard26 added skip news Does not need a NEWS file entry (eg: trivial changes) type: maintenance Related to Development and Maintenance Processes labels Dec 7, 2024
@ichard26
Copy link
Member Author

ichard26 commented Dec 7, 2024

Hmm, I wonder if the PSF Chronographer greyyed out status (from skip news) will still pass a required check branch protection rule...

@ichard26
Copy link
Member Author

ichard26 commented Dec 7, 2024

Hmm, I wonder if the PSF Chronographer greyyed out status (from skip news) will still pass a required check branch protection rule...

Yes, it does. When I temporarily update our branch protection rules to require the PSF Chronographer check instead of our own check, I can merge without complaint.

image

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

@ichard26 You may not know this, but this app was specifically written for this pip repo originally.
I made it when @pradyunsg was looking to rewrite the legacy browntruck bot that used to be integrated in pip. It mostly reuses the same logic but also grew extra features over the years and is being used in a bunch of places now.

The gray status is what GitHub Checks API has as neutral. Branch protection basically treats anything that is not failed or pending as a success in that context.

The check is a lightweight PR diff analysis that is much cheaper than invoking Towncrier.

You can lurk into the source @ https://github.com/sanitizers/chronographer-github-app. I'm specifically targeting configuration though an in-repo file to avoid hardcoding anything and keep it flexible.

As for why there are two simultaneous checks in the repo — I suggested keeping them side-by-side to note any differences: #11848 (comment).
And now it's time to drop the GHA variant.

@webknjaz
Copy link
Member

When I temporarily update our branch protection rules to require the PSF Chronographer check instead of our own check, I can merge without complaint.

Hm… I was under impression that it was supposed to be required already. Perhaps mark it that permanently already?

@pradyunsg
Copy link
Member

Let's do this!

@webknjaz
Copy link
Member

@pradyunsg I think it is expected that you click the actual approve button?

@ichard26 ichard26 merged commit d3ac6a2 into pypa:main Dec 10, 2024
11 checks passed
@ichard26
Copy link
Member Author

@webknjaz thanks for the additional context! It's appreciated.

@ichard26 ichard26 deleted the trim-unnecessary-gh-config branch December 10, 2024 19:18
@ichard26
Copy link
Member Author

To make it explicit: I changed our branch protection rules to require the PSF Chronographer status check (instead of our GHA news file workflow).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news Does not need a NEWS file entry (eg: trivial changes) type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants