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

MAINT: enhance the configuration for the pull-request-labeler workflow #19728

Merged
merged 6 commits into from Dec 22, 2023

Conversation

agriyakhetarpal
Copy link
Contributor

Reference issue

Closes gh-19088

What does this implement/fix?

This PR configures the pull request labeler workflow to run just once on a PR, i.e., at the time of its opening – with no subsequent runs on further synchronisation events or updates to said PR. The new release of actions/labeler@v5 has fixed a previous issue where the sync-labels input was not correctly configured (please see actions/labeler#112). The permissions for the workflow have been moved to the job for security reasons, considering that this workflow will contain just a singleton job which needs those permissions.

Additional information

This PR uses the pull_request_target event and therefore might not run with the context of the changes here; however, I had tested and debugged my changes to see whether they will work and put together an explanation in a PR on my fork of SciPy: agriyakhetarpal#2

 See scipy#19088. These permissions are moved from the level of the workflow to that of the job for security reasons, since this workflow uses the `pull_request_target` event.

[skip cirrus] [skip circle]
As per the discussion in scipy#19088

[skip circle] [skip cirrus]
See scipy#19088

[skip cirrus] [skip circle]
@agriyakhetarpal
Copy link
Contributor Author

Hi @lucascolley – I have pushed the changes you suggested in #19088 (comment), and would appreciate a review!

@lucascolley lucascolley added maintenance Items related to regular maintenance tasks CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure github Items related to the code repository labels Dec 21, 2023
Copy link
Member

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

thanks @agriyakhetarpal ! This LGTM, want to try merging @j-bowhay ?

The only part of the issue which this doesn't address is

Especially if reverted changes can be reflected that would be really nice

but I don't think it's worth trying to get that in now. I say we go with just labelling on opened for now and can look into that later if desired. It's quite rare that the amount of modules being touched decreases, and it's trivial to remove the labels manually.

@agriyakhetarpal
Copy link
Contributor Author

I agree with you, I don't know of a method to do that currently... my initial guess of a way to do that would be to write another job that needs the label_pull_request job and notes changes to the labels provided on a PR (additions, deletions, etc.) using which it puts out a message printing what was changed. Personally not a fan of this though because it would just add to the noise for all those who are subscribed to a PR.

See scipy#19088

[skip cirrus] [skip circle]

Co-authored-by: Jake Bowhay <60778417+j-bowhay@users.noreply.github.com>
@lucascolley lucascolley added this to the 1.13.0 milestone Dec 21, 2023
@j-bowhay
Copy link
Member

@lucascolley I cant remember the history on this but I am not sure merging this will enable the CI job?

@lucascolley
Copy link
Member

lucascolley commented Dec 21, 2023

I cant remember the history on this but I am not sure merging this will enable the CI job?

Yeah there's another switch to flip somewhere as https://github.com/scipy/scipy/actions/workflows/pull-request-labeler.yml shows "disabled manually". Do you have permissions for that?

image

We'll want to merge this before re-enabling the workflow.


x-ref #17116 (comment) for a possible follow-up as well as "if reverted changes can be reflected" as mentioned above

@agriyakhetarpal
Copy link
Contributor Author

Not sure about the reverted changes one. But for

One possible point for further improvement would be to pick off the acronym (eg. MAINT) and map this to a label

This should be possible at this time by accessing the title of a PR (note: this would require that the PR should be opened with the title since this workflow is going to run just once). Another option could be to pick off these acronyms from the commit messages – we can re-use the [Build wheels] logic from the workflow for building wheels and base off on that.


Please let me know what works, happy to debug on my fork!

Copy link
Member

@j-bowhay j-bowhay left a comment

Choose a reason for hiding this comment

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

Ok let's try this, can always disable again. Let wait to see that everything works smoothly and then can consider the suggested follow up

@j-bowhay j-bowhay merged commit 0f2a49f into scipy:main Dec 22, 2023
@agriyakhetarpal agriyakhetarpal deleted the enhance-pull-request-labeler branch December 22, 2023 14:45
@lucascolley
Copy link
Member

thanks for your first contribution to SciPy @agriyakhetarpal !

@agriyakhetarpal
Copy link
Contributor Author

And thanks for your support, it's much appreciated!

@j-bowhay
Copy link
Member

j-bowhay commented Dec 22, 2023

Looks like something isn't working quite right, will disable for now to save resources
https://github.com/scipy/scipy/actions/runs/7304163132

@lucascolley
Copy link
Member

Looks like we just need to update label-globs.yml, will submit a PR

@adityaraute
Copy link

Whoa, big one this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Items related to the CI tools such as CircleCI, GitHub Actions or Azure github Items related to the code repository maintenance Items related to regular maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pull-request-labeler misbehaving and therefore disabled again
4 participants