[IgorPartola]

Ugh. Pull requests again? OK, look, unless your team is so large that you can't effectively communicate otherwise, pull requests are just friction. Use feature branches, then merge them when they are ready. Trying to pretend like your two person team is a huge open source project with many distributed part-time and full-time developers is cargo cult at its worst.

[jaegerpicker]

I couldn't disagree with this more. Pull requests provide a quick and relatively pain less code review tool. Unless you are suggesting that code review is a bad practice, which in that case I'm pretty well dumb founded as I think it's a requirement for intelligent modern development.

[IgorPartola]

They are a great tool for when you don't have any other way to communicate with the other developers. Code reviews are a great practice, and we all should do more of them. I do it when I get an email like "hey, could you review my code in feature-branch-xyz before I merge it?" It's less friction than doing GitHub-proprietary pull requests, and involves much less pompousness.

[jaegerpicker]

I guess different environments lead to different flows being better but managing code reviews via email sound really horrible to me. There is no good way to track who has actually reviewed the code, no easy way to annotate the code, and no easy way to give the code a seal of approval. Both Bitbucket and github provide good methods that are easily tracked for this. It' one thing to send an email saying please look at this code but It's much more difficult to track who on that email has done the review and who approves the code.

We use feature branches and issue pull requests/code reviews from those feature branches in BitBucket and it's been a universally well received tool and definitely increased the quality of our code reviews and overall code base.

[IgorPartola]

In my original comment I mentioned a setup with a two person team. You know exactly who is reviewing or not reviewing your code in this setup: the other person. The two of you cannot possibly produce so many feature branches that you don't remember your own code that has not yet been reviewed.

If the PR method works for your team, that's great. In my experience, it leads to more friction and the reasons to implement it are usually "best practices" articles like TFA, not actual needs of the team.

[sanderjd]

I really like having code review persisted right there in the repository. I often go back and look at discussions on old pull requests to figure out why we decided to do something one way or another. It's even "higher resolution" than commit messages (which should also be good).

[hoggle]

We are a trusting team and don't do pull requests with our closed source repos either.

To avert chaos it's much better to have an orderly branching model in place e.g. feature branches! My fellow partners made us go all "git flow" last year and the result has been a really tidy and satisfying workflow:

http://nvie.com/posts/a-successful-git-branching-model/

Atlassian's SourceTree also helps us with maintaining "the flow":

http://www.sourcetreeapp.com

https://www.atlassian.com/git/workflows

Git extensions and screencast on how to setup on OSX:

https://github.com/nvie/gitflow

http://build-podcast.com/git-flow/

[xtracto]

We have been using Git Flow in our team too. One advantage I se is that using git-flow specific hooks we add some metadata to every time a feature/ branch is finished. This metadata goes into an automatic Release notes file which is automatically added to the repo (as an amend of the flow's last commit).

Similarly, hotfixes have metadata which goes to their own hotfix.csv file.

A deploy script processes the hotfixes and feature files to create a Release notes.

We've got a very smooth release documentation process which is difficult for Dev's to hate and skip.

[buffportion]

Or don't use branches at all. Develop on trunk/master with continuous integration. It reduces work-in-progress, exposes conflicts sooner and has a host of other knock-on benefits.

[chrisweekly]

To 'CI' [per se] I say "yes, of course." It's a best practice. But it is not a substitute for branching! I want to choose when I'm exposed to conflicts. "Sooner" is usually -- but not always -- "better". Summary: strong disagreement on this radical approach.

[buffportion]

I don't consider not-branching to be a radical approach. If you are using branches you really aren't doing CI, because you aren't integrating continuously.

[joshuacc]

> If you are using branches you really aren't doing CI, because you aren't integrating continuously.

Perhaps this is just a terminology issue, but I strongly disagree with this. You can always merge/rebase the latest version of master into your feature branch.

[buffportion]

That will integrate other people's changes into your branch. But until you also merge your branch into master (and/or all other branches), you are not integrating your changes.

[IgorPartola]

So there are projects where I don't branch: ones where I am the only committer, and there are no features. For example, my puppet manifests for all the servers I manage. It either works, or I roll it back.

However, feature branches are just multi-stage commits. Sure you can invest hours of your life setting up CI, then writing code, not testing it locally before pushing it to master, getting an obscure error from the test/deploy process, fixing it again, pushing again, getting another error, etc. But you probably want to, you know, ship the product.

Having said that, if your workflow actually resulted in you writing better code faster and you ended up shipping earlier and making more money, I'd love to read about it.

[buffportion]

"Sure you can invest hours of your life setting up CI, then writing code, not testing it locally[...]"

Maybe I've misunderstood, but why aren't you testing it locally? With trunk based development you make your changes, run a local build and push if it passes. Everyone (including any CI servers) runs the same build process.

The trick is that you have to be able to push to master without breaking it, which is scary to many developers and usually requires a shift in thinking. How do I do large refactorings? (you don't - do many small refactorings instead). How do I add a half-finished feature? (break the feature into many smaller features, use feature toggles, etc.)

[IgorPartola]

Of course. And this relies on your CI system having 100% unit and system test coverage, including testing external API's, CSS bugs, browser compatibility, etc. Are you certain that your system does that? More importantly, are you certain that investing the time into testing that your checkout button is not covered up by some random element due to a missing ";" character in your CSS file or a missing 0 in the width percentage value will eventually pay off? It should be scary to push to master if you hold master to be deployable at all times. Unit/system testing or not, you will never have guaranteed test coverage, and even if you try to get to it, you will spend infinitely more time doing that, than doing a reasonable amount of unit/system automated testing + good human QA.

[nostrademons]

Both Google and Facebook use trunk-based development. If Google can get away with a single trunk and 15,000 or so committers, chances are your organization can too.

The reason for trunk-based development over feature branches is that the time required to integrate a patch usually increases proportional to the square of the time since last integration. With trunk-based development, most commits reflect about 1-2 days worth of work, and they go in with minimal conflicts. Branches (and patches that sit idle for a month or two) often take forever to integrate, because as you are trying to update your code to the new master, the master is itself changing. At some point, the process doesn't converge, and you can spend forever trying to update a branch.

[auxbuss]

This is my preferred method too. But a lot of developers struggle with the discipline of small, discrete, incremental changes, plus the dependence on solid testing skills. It also requires a culture that can support it. No blame, in particular.

However, if you have a team that is comfortable working this way -- and is only occasionally breaking things; it happens -- then it is a wonderfully creative, frictionless, and flexible way to develop.

[chris_mahan]

That's what we do.

Works well.

Break the build, all stops, and we huddle and fix it.

Sort of like the andon cord in toyota factories.

[angersock]

This is a masterful troll, right?

...please?

[buffportion]

I appreciate the backhanded compliment but my intentions are pure :-)

[awj]

Meh, it doesn't have to be friction. If the goal is to ensure your other team member sees the changes you're making before they're merged in a pull request both accomplishes that and gives you both a place to put some notes related to that process.

I think you're ascribing too much ceremony to this. It's useful to have something written down for any discussion about these changes, and a pull request is as good of a place as any for that. Plus it ensures everything is all set up to merge once you're both satisfied with the changes.

[eropple]

While I'm often lazy about this, I think that there are advantages to doing this with this once your team is over just a solo developer. Encouraging your fellow developers to read (and at least understand, if not internalize) your code has some bus-proofing to it, and I don't think it's that much friction if done well.

[dllthomas]

And helps make sure people are familiar with more of the code.