[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.

[thelastnode]

I like to put a summarizing comment in the code for lines that require justification like the ones you mention. That way the discussion happens on GitHub and the conclusion is in the code, where future developers will see it.

[diggan]

A good thing to think when you start adding documentation to the code is "Am I really writing readable and understandable code right now?". If you answer isn't yes without any hesitation, you usually need to refactor so other people can read your code instead of you needing to comment what you do.

[thelastnode]

True, but sometimes there isn't a choice, e.g., working with a clowntown library, poorly designed legacy code, etc.

[prostoalex]

http://phabricator.org/ covers precisely that - code reviews and discussions around commits. The recommended installation includes Arcanist command line tool, which modifies Git commit template to include a URL of the Phabricator code review, test plan, and reviewers' names.

[alexchamberlain]

Hmmmm I'm not sure it does. The problem (I have experienced) with Phabricator is that it only operates on the diff you upload, so it can lack context of the project as a whole.

[ndr]

If the change is smalle enough you should be able to understand what that set of commits were for by the name of the branch.