Howdy Miller and all,
Just some quick thoughts on git & github workflows for Pd.
Github is useful in that it makes contributing to a project's codebase easier without needing direct access using it's "Pull Request" mechanism. AFAICT, this is something *possible* using just git, but GitHub makes it relatively *painless*. I think it's great to see, at this point, many more people now contributing to Pd but, of course, how do we handle this?
Some observations/ideas:
1. Master branch as develop: The master branch is kept the same between the sourceforge "upstream" and the GitHub "mirror." So far we've managed to keep that true and Miller is the only one moving work from GitHub branches and PRs into "upstream."
2. Release schedule: Since we're on a yearly release schedule, with (possible) bug fixes after about 6 months, PRs and branches now build up on Github in the meantime. This is good. Mostly, we've managed to keep these relatively isolated from each other so they can apply cleanly. This is not always possible, however, for some things (for example, the recent const qualifier additions).
3. Merge order: I think what's important, then, is merge order. The smaller PR's that only touch unconnected files should be merged first. The larger PR's that touch *many things* should be merged later and, only after their author's have updated their branch by merging it with the now updated master. This should make things merge cleanly.
4. Using Github: The PR mechanism on Github also tools to gauge how the proposed changes will affect the codebase. The diffs and commit listing is useful as well as the result of the continuous integration build that IOhannes set up. You can basically see, at a glance, if the PR will merge cleanly or not. If everything is good, the easiest solution is really to use the green "Merge" button on the PR page, then pull those changes from the Github mirror to the upstream sourceforge repo. The push and pull can go *both* ways, with the addition of using the tools that make this easier. (If anyone is in the "I hate GitHub" camp, the same workflow is true with using Merge Requests in Gitlab.)
5. Feedback: As PR authors, we want to help you, Miller! If you're considering merging some work but want a few changes, no problem. Let us know in the PR discussion and someone will probably do it. Then we can make sure the PR branch is up to date and will merge cleanly. Again, the green "Merge" button is easy and useful here. Ideally, all you would need to do to integrate most things, is just hit "Merge" a few times and then pull the changes for the Github mirror.
6. Making changes: In my own Github projects, oftentimes someone gives me a PR which I want, but I will probably change a few details here and there. I usually handle this by merging it on the PR page, pulling, then making a cleanup commit later. This is usually easier than cherry-picking some commits and manually applying parts of others. Again, this works well for small things but not for larger stuff. In the latter case, giving feedback until you are satisfied with the proposed diff is the way to go.
7. Testing before merging: This is possible by adding the PR author's repo as a remote and then pulling whatever branch (master or otherwise) they submitted the PR with. This allows you to essentially do a checkout of their PR branch that you can build locally. I haven't needed to do it myself very often, but it has been useful in providing feedback and updates. There is also a mechanism to make commits to *their* branch before actually accepting the PR, but that's something more GitHub specific through their authentication setup.
-------- Dan Wilcox @danomatika http://twitter.com/danomatika danomatika.com http://danomatika.com/ robotcowboy.com http://robotcowboy.com/
A small followup:
8. Changelog help: It's hard to write a change log when there are lots of contributions. Something helpful is to see the commits between now and the last release tag, then copy/paste from there, for example:
git log HEAD...0.48-2 > log.txt
will output all commit messages from the current master HEAD commit back to the 0.48-2 release tag.
I believe the openFrameworks teams uses a variant of this approach.
On Sep 11, 2018, at 12:49 PM, Dan Wilcox danomatika@gmail.com wrote:
Howdy Miller and all,
Just some quick thoughts on git & github workflows for Pd.
-------- Dan Wilcox @danomatika http://twitter.com/danomatika danomatika.com http://danomatika.com/ robotcowboy.com http://robotcowboy.com/
On 2018-09-11 12:49, Dan Wilcox wrote:
- Testing before merging: This is possible by adding the PR author's repo as a remote and then pulling whatever branch (master or otherwise) they submitted the PR with. This allows you to essentially do a checkout of their PR branch that you can build locally.
personally, i always do a local investigation of PRs before merging them (unless the changes are trivial). so i practically never use github's "merge" button.
however, i've found that adding the PR author's repos as remotes quickly gets painful, the more PRs there are, as it adds all the branches of the PR author's repo to mine. and i tended to forget to remove them.
so i was *very* thankful when chr13m pointed me to a solution to only pull a specific PR in [1].
later i learned that there's a "git pr" command (on Debian this is shipped in the "git-extras" package; i don't know about Fedora), which does essentially the same:
e.g. to locally fetch PR#440, i just cd into my pure-data.git dir and run: ~~~ $ git pr 440 ~~~
fgasdmr IOhannes
[1] https://lists.puredata.info/pipermail/pd-dev/2018-06/021591.html
On 11/09/18 20:55, IOhannes m zmoelnig wrote:
personally, i always do a local investigation of PRs before merging them
For those who find the presence of GitHub's green "merge" button to be a deeply offensive abomination[1], there is this userscript (which you can install with Tampermonkey or similar) for hiding it automatically:
https://news.ycombinator.com/item?id=3960876
Cheers,
Chris.
[1] https://github.com/torvalds/linux/pull/17#issuecomment-5654674
On 12/09/18 10:54, Chris McCormick wrote:
On 11/09/18 20:55, IOhannes m zmoelnig wrote:
personally, i always do a local investigation of PRs before merging them
For those who find the presence of GitHub's green "merge" button to be a deeply offensive abomination[1], there is this userscript (which you can install with Tampermonkey or similar) for hiding it automatically:
Here is the correct link:
https://gist.github.com/aripollak/4322621/
Cheers,
Chris.
On 2018-09-12 04:54, Chris McCormick wrote:
On 11/09/18 20:55, IOhannes m zmoelnig wrote:
personally, i always do a local investigation of PRs before merging them
For those who find the presence of GitHub's green "merge" button to be a deeply offensive abomination[1], there is this userscript (which you can install with Tampermonkey or similar) for hiding it automatically:
rofl.
gasmdr IOhannes
This is essentially how I do it too... for PRs on the pure-data project it's like this:
a=<name of branch> git fetch https://github.com/pure-data/pure-data.git $a git checkout -b $a FETCH_HEAD git checkout -b tempmerge master git merge $a
If the temporary merge seems to work (or if I can fix it up so that it works) I can then merge it back into master; if I get hung up in merge problems I can delete the branches and it's as if I didn't do anything.
I started doing this when I tried to merge the memory leaks/management PR and ended up having to do a lot of conflict resolution by hand, then gave up in the middle of the process but couldn't figure out how to unwind the changes I had made to master. (There are still buts of code floating around from that, so I've also made it much harder to ever apply that PR in the future).
I think of git as a kitchen blender, very powerful when used correctly, but if you've ever started one without remembering to put the top on you will also learn of its power to spread mayhem.
cheers Miller
On Tue, Sep 11, 2018 at 02:55:15PM +0200, IOhannes m zmoelnig wrote:
On 2018-09-11 12:49, Dan Wilcox wrote:
- Testing before merging: This is possible by adding the PR author's repo as a remote and then pulling whatever branch (master or otherwise) they submitted the PR with. This allows you to essentially do a checkout of their PR branch that you can build locally.
personally, i always do a local investigation of PRs before merging them (unless the changes are trivial). so i practically never use github's "merge" button.
however, i've found that adding the PR author's repos as remotes quickly gets painful, the more PRs there are, as it adds all the branches of the PR author's repo to mine. and i tended to forget to remove them.
so i was *very* thankful when chr13m pointed me to a solution to only pull a specific PR in [1].
later i learned that there's a "git pr" command (on Debian this is shipped in the "git-extras" package; i don't know about Fedora), which does essentially the same:
e.g. to locally fetch PR#440, i just cd into my pure-data.git dir and run:
$ git pr 440fgasdmr IOhannes
[1] https://lists.puredata.info/pipermail/pd-dev/2018-06/021591.html
Pd-dev mailing list Pd-dev@lists.iem.at https://lists.puredata.info/listinfo/pd-dev
On 9/12/18 7:59 PM, Miller Puckette wrote:
This is essentially how I do it too... for PRs on the pure-data project it's
btw, it seems that you accidentally pushed some helper-tags, namely "after-infinite-undo" and "before-inf-undo". i've removed them from the github repository, but you might want to remove them locally as well, to prevent re-pushing them.
gfasmrd IOhannes
On 13/09/18 01:59, Miller Puckette wrote:
I think of git as a kitchen blender, very powerful when used correctly, but if you've ever started one without remembering to put the top on you will also learn of its power to spread mayhem.
Lol very apt!
A particularly handy command when you get stuck is the `git reset` family. If everything gets hosed you can return your branch to some previous (or future) point in history by going `git reset --hard REVNO` where REVNO is the hash of the commit you want to reset everything to. This will delete any modifications you've made in local, so often it's wise to do a `git stash` first if you have local modifications.
Cheers,
Chris.
-
Chris McCormick -
Dan Wilcox -
IOhannes m zmoelnig -
IOhannes m zmölnig
-
Miller Puckette