Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Useful GitHub patterns

blog.quickpeople.co.uk

Post reply on HN
Print view View original

11–20 of 21 posts

Re: Useful GitHub patterns

#11

Post by prostoalex »

Timothee wrote: post #4

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 t…

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.

Re: Useful GitHub patterns

#12

Post by alexchamberlain »

prostoalex wrote: post #11
Timothee wrote: post #4

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 t…

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.

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.

Re: Useful GitHub patterns

#13

Post by mineo »

Timothee wrote: post #4

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 t…

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

Re: Useful GitHub patterns

#14

Post by ndr »

Timothee wrote: post #4

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 t…

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.

Re: Useful GitHub patterns

#15

Post by tomgirl1 »

GhotiFish wrote: post #3

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 educat…

I agree, I am a big fan of github, but the more I understand git in general, the less fond I am of github's evangelism.

I'm definitely not a git expert but I did shutdown my github account once I had the feel of it, since in reality I dont need github to use git effectively or collaborate with others.

That said I think github is great for open source collaboration, but I dont think it's going to be the last word in that space either.

Re: Useful GitHub patterns

#16

Post by 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)

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

Re: Useful GitHub patterns

#17

Post by wizard_2 »

cmwelsh wrote: post #8

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?

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.

Re: Useful GitHub patterns

#18

Post by Timothee »

mineo wrote: post #13
Timothee wrote: post #4

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 t…

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 co…

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.

Re: Useful GitHub patterns

#19

Post by icedchai »

cmwelsh wrote: post #8

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?

curious. what industry are you in?

Re: Useful GitHub patterns

#20

Post by cmwelsh »

wizard_2 wrote: post #17
cmwelsh wrote: post #8

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?

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.

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.
Post reply on HN