Switch howto_contribute to a Github workflow. #3995
Conversation
|
I think we'll need to do |
mateor
changed the title
There was a problem hiding this comment.
Testing the github review process. I don't see a 'Fix it and then Ship it" analogue.
It was nice to have two classes of comments
- suggestions
- prerequisites for a Ship It
I guess the could be split into two reviews? It appears that comments must be classified as a group.
Also, I was able to change the text of the pull request title! I capitalized 'github' to test if the 'edit' button actually allowed me to do that. Crazy.
| @@ -90,18 +84,15 @@ committed. | |||
|
|
|||
| If you've already cloned your repo without forking, you don't have to | |||
| re-checkout your repo. First, create the fork on github. Make a note the | |||
There was a problem hiding this comment.
"Make a note of the clone url"...
| @@ -115,13 +106,14 @@ You can always run the pre-commit checks manually via: | |||
| :::bash | |||
| $ ./build-support/bin/pre-commit.sh | |||
|
|
|||
| **Pro tip:** If you want your local master branch to be an exact copy of | |||
| **Pro tip:** If you are certain that you have not accidentally committed anything to | |||
There was a problem hiding this comment.
I might just remove the Pro tip entirely - seems out of scope for the Pants docs.
| them with a link manually or they see it in the google group. | ||
| Note that while only committers are available to add in the Assignees field, | ||
| any pants contributors may still post reviews if you provide them with a link | ||
| manually or they see it in the google group or github notifications. |
There was a problem hiding this comment.
should probably capitalize all 'github' references.
| :::bash | ||
| $ git push origin master | ||
| A committer should push the `Squash and merge` button on the PR, and ensure that the | ||
| commit message generated from the review summary is accurate. In particular, the title should |
There was a problem hiding this comment.
Yuji points out on the rbcommons review that we should have some kinds of guidelines/template for the PR summary. It would be nice if it looked something like the RBCommons one, but of course we can leave out the RBCommons link. I wonder if the github PR number gets automatically written into the changelog?
There was a problem hiding this comment.
Noting that at a small defeat to the drive towards fully hosted infra, we can write a check webhook that we force to be green for a commit to be merged. That we could run on Amazon and it could check commit messages, etc to our content. We could pair that with a PR creation script as well as a merge script to make this easier-ish to get right from the reviewee and reviewer ends. Lots of "we" in there and I'd be happy to be the we who gets this all setup if folks agree on what to check, etc.
There was a problem hiding this comment.
I tested and - at least with only the squash button enabled, it is added at the end of the subject line like so (#1) - that's for PR #1.
|
@jsirois : Regarding branch protection: I was going to suggest it, but I was concerned that it would prevent commits to the pushdb. Maybe it's an opportunity to improve how that works to make it optionally branch-based? Might align better with the transactionality of sonatype anyway. But: we don't currently have any sort of branch protection, so it also feels like a new feature. |
|
Yeah - definitely a new feature, but I also imagine alot of mess-ups using this new system. I hadn't thought about the pushdb - we could allow admins to push w/o review and make sure admins == folks who can publish to sonatype - not ideal. |
stuhood
changed the title
|
Merged as e35dc2e |
Problem
As discussed at a few summits in a row, rbcommons is painful (and costly) roadblock for new contributors. #3708 suggested reviewing review tools to find an alternative.
Solution
This PR updates the documentation and scripts to switch to a github workflow.