> On May 31, 2017, at 8:52 AM, Zero King <[email protected]> wrote: > > On Wed, May 31, 2017 at 10:40:07AM +0200, Mojca Miklavec wrote: >> The new bot could solve some of those problems, but it's still a pity >> that maintainers cannot have slightly higher permissions set. > > What kind of slightly higher permissions? I can make the bot auto-merge > PRs approved by all related maintainers but that sounds dangerous. > >> In theory GitHub's pull requests should allow to have *much less* >> committers. In theory doing the reviews and merging pull requests >> should be much easier that doing the same thing on Trac where the >> patches get outdated, cannot be reviewed on line-by-line basis etc. In >> practice I need to have a cheatsheet for merging pull requests and do >> some not-anywhere-easy-to-remember steps to be able to merge trivial >> PRs when some modifications are desired. > > I wrote a Gist about making changes to PRs, feedback via email is > welcome. https://gist.github.com/l2dy/7da9621954ebcf1a19869f391662a41e
I currently know very little about the GitHub pull request process. I’d like to understand better and this write-up helps a lot. Some observations, ... 0) The wiki currently includes the following: https://trac.macports.org/wiki/WorkingWithGit I presume we would adapt your Gist to become something like “WorkingWithGitHubPullRequests”? I think the current page would be unweildy if the gist was tacked on. 1) Gist lines 1-18 are similar to the existing Basic Setup and Cloning a Repository sections of the WorkingWithGit (WWG) wiki page. Could we avoid repeating by modifiying the existing page? 2) I like your Disaster Recovery sections. I wonder if we could use wiki formatting to make them standalone boxes with emphasis. 3) Around line 24, I think there should be some preamble giving an overview of the typical workflow for a PR. Ie, after the PR is initially proposed, the initiator and review may go back and forth a number of times amending the PR to address issues, etc. When ready, the intermediate commits are squashed (I don’t know GitHub well, is this the right term?) so that the history in MacPorts doesn’t include extraneous commits. I don’t see how this is addressed in the gist. 4) Line 101-103 - I don’t understand why one would need to do this? After a PR has been commited, wouldn’t one just delete the local branch? Craig
