
Useful GitHub patterns - benilov
http://blog.quickpeople.co.uk/2013/07/10/useful-github-patterns/
======
Timothee
_GitHub’s functionality around PRs (such as inline commenting, replies,
notifications and diffing) is excellent for facilitating code and design
discussion_

I agree but one thing I've realized recently is how all the history and
discussion happening in a PR tend to be lost once the PR is merged. Not
completely lost because it's still there in the "closed" PRs, but
undiscoverable.

For example, you come across one line that doesn't make sense to you but you
sense that there's a reason behind it. 'git blame' will tell you the specific
commit but the commit message might not be explicit and you won't be able to
find the corresponding pull-request easily.

I mentioned it on Twitter and exchanged with a GitHub employee that seemed to
understand my issue. Hopefully they'll improve on that…

~~~
mineo
Imho GitHubs pull requests shouldn't be used as a replacement for good commit
messages or code comments - GitHub might go away, you might decide you don't
like them anymore or whatnot. If you feel like someone raised a question about
some part of your code in a PR that does have an answer that's only obvious to
you, there's no harm in just adding another commit adding comments to the code
(you can even squash that commit into the commit changing the code between
merging to master and pushing).

~~~
Timothee
I agree. It just doesn't always happen. I'm guilty of leaving too-short commit
messages and not enough commenting as much as some of my teammates.

------
GhotiFish

      4. the sneaky commit
        when do I use it?
    
        after the code has been reviewed and merged into master
        I need to make a small change (eg a copy change or 
        bugfix) that’s not even worth notifying others about
    
      what I do
    
        just push the new commit to master.
    

ಠ_ಠ That's not something you're supposed to admit in public.

I will note that github has being particularly prolific in getting git advice
out there, they're working to educate developers in git, full stop. Though one
or two pieces (such as this one) seem to suggest it's github specific advice.

------
dbaupp
> _GitHub deals well with force-pushing to a PR branch, ie it doesn’t lose the
> comments on the previous commits, etc_

This has not been my experience at all, it seems to loose non-line comments
always, and line comments sometimes.

\---

In any case, I've been doing a lot of work on the Rust language recently, and
there is an integration bot ("bors") which detects `r+` comments on the last
comment on a pull-request, then runs 13 different configurations of the full
test suite (4 platforms, 2 architectures, optimisations on/off, with/without
valgrind; although not all combinations of these). If (and only if) all 13
test runs pass, the bot automatically pushes the commits to master.

(Almost) all commits go through bors, with increasingly less common pushes
straight to master to get the test suite to pass again after some breakage.
(The test suite used to only with 3 configurations for each pull request, so
breakage occurred semi-often.)

(An example,
[https://github.com/mozilla/rust/pull/7693](https://github.com/mozilla/rust/pull/7693))

When it works, it is really nice; get someone to review, they approve it and
the whole process is managed from there, no mistakes possible. (However,
apparently the GH api is unreliable, so bors has moments of madness.)

------
cmwelsh
My employer is similar but a little stricter about how we use GitHub pull
requests. All new code goes in via pull request. All pull requests are
reviewed by another developer. Almost all pull requests are sent to QA to test
acceptance criteria.

The sneaky commit will get you many frowny faces on chat.

It works rather well. Does anyone else work like this?

~~~
wizard_2
My team works like this with a twist. You merge your own pull request. You
have developers, designers, pm's, etc to help review the change. You use them
if you need them and you take responsibility for your bugs. We'll drop
everything to review someone else's pull, and in turn that happens for your
own pulls.

~~~
cmwelsh
We also merge our own pull requests but there is less concept of code
ownership. Bugs can be fixed by anyone who grabs them and assigns them to
themselves. Blame may point out the cause and you can send the author a
friendly message on Skype so they are aware of your fix. Perhaps they can even
mark your fix as peer reviewed while they're at it.

------
d4mi3n
This was actually a pretty decent read. The author lists a number of common
use-cases for Github pull requests and how they can be used in practice.

