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

actions: add maintainers as PR reviewers for their packages #12269

Merged
merged 1 commit into from Aug 4, 2019

Conversation

tgamblin
Copy link
Member

@tgamblin tgamblin commented Aug 4, 2019

This adds a GitHub action that uses spack pkg changed and spack maintainers to figure out which packages changed and who their maintainers are in a PR.

Then it adds any maintainers to the PR as reviewers.

@tgamblin tgamblin force-pushed the features/maintainer-action branch 2 times, most recently from a2f44a8 to 6916860 Compare August 4, 2019 01:22
@tgamblin
Copy link
Member Author

tgamblin commented Aug 4, 2019

Outstanding question: should we also add a comment that says something like "adding the following package maintainers as reviewers on this PR"?

@tgamblin
Copy link
Member Author

tgamblin commented Aug 4, 2019

Also, I have no idea how to write tests for this.

Use `spack pkg changed` and `spack maintainers` to figure out which
packages changed and who their maintainers are in a PR.

Add any maintainers to the PR as reviewers.
@tgamblin tgamblin changed the title actions: add GitHub action to add maintainers to packages actions: add maintainers as PR reviewers for their packages Aug 4, 2019
@adamjstewart
Copy link
Member

should we also add a comment that says something like "adding the following package maintainers as reviewers on this PR"?

That would be pretty cool, but not absolutely required.

We need to start being more aggressive with recommending that people become package maintainers. At the very least, we can add it to the spack create template. I think in conda-forge, every package is required to have 1+ maintainers.

@tgamblin
Copy link
Member Author

tgamblin commented Aug 4, 2019

That would be pretty cool, but not absolutely required.

Ok -- let's merge this, then, and we'll get the initial workflow going.

We need to start being more aggressive with recommending that people become package maintainers. At the very least, we can add it to the spack create template. I think in conda-forge, every package is required to have 1+ maintainers.

I think if it actually does something (like this PR does) they'll be more motivated.

@tgamblin
Copy link
Member Author

tgamblin commented Aug 4, 2019

Note that I still need to make a main.workflow and stuff to tell this action to run -- this is just the Dockerfile and entrypoint script.

@tgamblin tgamblin merged commit 3179d98 into develop Aug 4, 2019
@adamjstewart adamjstewart deleted the features/maintainer-action branch August 4, 2019 14:18
@tgamblin tgamblin mentioned this pull request Aug 4, 2019
@ax3l
Copy link
Member

ax3l commented Aug 6, 2019

btw, could one re-use parts of the following GitHub feature? could also be too cumbersome for our use (e.g. edit in two places).
https://help.github.com/en/articles/about-code-owners

@tgamblin
Copy link
Member Author

tgamblin commented Aug 6, 2019

@ax3l: did you see my note in the original PR?

@tgamblin
Copy link
Member Author

tgamblin commented Aug 6, 2019

Hm I can’t find the nite. Yes it’s like codeowners but I don’t want people to have to know about the paths to the packages or edit two files — easier if they do it in the And potentially on generated websites and things, whereas codeowners doesn’t.

dev-zero pushed a commit to dev-zero/spack that referenced this pull request Aug 13, 2019
)

Use `spack pkg changed` and `spack maintainers` to figure out which
packages changed and who their maintainers are in a PR.

Add any maintainers to the PR as reviewers.
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.

None yet

3 participants