The contribution bot currently makes a comment on pull requests from core contributors to their repos:
Thanks for the pull request, @bradenmacdonald! As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.
Are these still useful? There’s a label (core committer) that’s applied also, but will soon stop. Are those useful? The data about who is a core committer and where they can commit is out of date, and we don’t have a plan how to keep it current.The larger question here: what should the bot do to help with core committership?
I don’t think the comments are useful. CCs already know which repos they can merge to, and so I don’t think the bot needs to remind them?
Keeping these labels (and maintaining the core committer data that drives them) could be useful though, if we’re tracking the number of PRs created and merged by core contributors. That can be determined retroactively of course, but since the CC people and their repos change over time, these tags provide a “snapshot in time” which might be harder to store and track in another way.
Whether this effort is worth the above… I don’t know.
I think the bot could help PR authors outside of the CC program (and outside of the edX/2U/Axim org) get their changes reviewed faster.
For example, the bot could add a message to their PRs simply linking to the current list of core committers, so they can find people to ping about getting changes reviewed. Or if we maintain the CC/owner+repo data for use by github, the bot could @mention the relevant people and/or teams directly.
@jill In the context of OSPR management we’ve standardized on the new(-ish) core contributor label. This label is being added to PRs as appropriate by @Michelle_Philbrick and me when triaging open source contributions via the contributions board.
We use the official Core Contributors directory to check if a PR author is a core contributor, and will add the core contributor label irrespective of whether a PR author has core contributor status for the target repo of their PR. See this comment (and the full conversation starting here) for details.
What this means is that the core committer label that @nedbat mentioned has become somewhat redundant, and we don’t really need it for the purpose of tracking PRs from core contributors
Automating some parts of the review process to help speed things up is a good idea. We would probably need to think about how to make things like pinging potential reviewers happen at the right time, though – usually there are some steps that need to be completed before an OSPR is ready for engineering review. (For example, first-time contributors must submit a signed Contributor Agreement, and tests must be manually enabled for their PRs after determining that they don’t contain any malicious code; PRs affecting end-user experience must go through product review; etc.)
As of today, the bot no longer adds a “core contributor” label (unsure of the spelling atm). Also, sorry that the bot was offline for a few days there, but all should be caught up now.
I think some sort of comment is useful but that wording needs to be updated. I think it’s more useful for the reviewer to know whether they need to merge the PR or the person who opened it will merge it.
I agree this would be good info for reviewers to have. We would need to put in some work to make it happen.
Currently, the bot knows accurately who is a Core Contributor, based on an export from Axim’s Salesforce data. However, its knowledge of who has write access to what repo is outdated, as it is using the people.yaml file (private link) which is not being updated regularly.
We would either need to start updating people.yaml again, or we would need to teach the bot to read the CC<->repo data from somewhere else, such as Salesforce or GitHub directly.
As of last Friday afternoon, the bot does nothing for core contributors. The data in people.yaml was stale and unmaintained, and we have no planned work to duplicate it in Salesforce. When we get that information in a new machine readable form, we can add back the core contributor commenting.
