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

feature: add maintainers from build_systems to new packages #84

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tldahlgren
Copy link
Collaborator

@tldahlgren tldahlgren commented Apr 26, 2023

Fixes #90

It gets really annoying to add "inherited" maintainers as reviewers to new packages.

This PR performs a simple maintainer account extraction from build system packages and ensures they are added to the maintainers list.

To test this, we need to add a new package that inherits from python, ruby, or racket (but python is the easiest and most important). We need to make some changes to support easier testing of spackbot changes in staging, and then we can verify this is working and merge it.

*The logic has been tested manually outside of the spackbot context.

@tldahlgren tldahlgren requested review from tgamblin and alalazo April 26, 2023 01:44
@github-actions
Copy link

Please format your code with black: black spackbot.

@tldahlgren
Copy link
Collaborator Author

ping @scottwittenburg Any chance of getting this reviewed and maybe merged "soon"?

@scottwittenburg
Copy link
Collaborator

Yes, sorry I just lost track of this. I'm traveling this week, and since it looks like this needs testing before deployment, I won't be able to get to it until next week. Unless @zackgalbreath or @kwryankrattiger can review and test in the meantime.

@kwryankrattiger
Copy link
Collaborator

kwryankrattiger commented Jan 16, 2024

The code itself seems fine. I would like to test drive this in testing repository just to make sure it is working as expected though.

Or if you have a link to your manual testing that works too.

@tldahlgren
Copy link
Collaborator Author

The code itself seems fine. I would like to test drive this in testing repository just to make sure it is working as expected though.

Or if you have a link to your manual testing that works too.

I don't have a link. 🙍

@alalazo
Copy link
Member

alalazo commented Mar 5, 2024

@kwryankrattiger What's the status on this? Would save us a lot of clicks each day 🙂

@kwryankrattiger
Copy link
Collaborator

Still waiting on the ability to test things afaik.

@alecbcs
Copy link
Member

alecbcs commented Mar 5, 2024

@kwryankrattiger do you need access to a specific repo/github app? Or documentation on how to setup a test version of Spackbot?

@kwryankrattiger
Copy link
Collaborator

We have a workflow for testing the robot, however it is quite cumbersome to set up and only one person can test at a time. These restrictions makes it extremely difficult to do testing. We talked about this a few weeks ago and the goal is to find a way to let people test the robot themselves rather than asking Scott or I to do the testing. I think @scottwittenburg was putting something together at one point but I can't remember the status.

@scottwittenburg
Copy link
Collaborator

I have on my list moving the current development spackbot into our staging environment, but I haven't gotten to it yet. Also, I haven't thought about how to get from there to where the rest of the workflow for spackbot contributors to test their changes is as easy as possible.

An option is that we just merge this change because of its high impact, and then just monitor for breakages and either quickly fix or revert.

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

Successfully merging this pull request may close these issues.

Maintainers are not notified for new packages
5 participants