Live data from Hacker News

A better pull request

developer.atlassian.com

31–40 of 167 posts

Re: A better pull request

#31
post #4

Earlier quoted context omitted.

But still the test would only fail after the merge, whereas you'd want to catch this before.

On GitHub you can have tests run before you've merged, so you know whether it's safe or not.

Yeah - we run both the pull request and the master + branch merge every time.

Re: A better pull request

#32
post #4

Earlier quoted context omitted.

But still the test would only fail after the merge, whereas you'd want to catch this before.

On GitHub you can have tests run before you've merged, so you know whether it's safe or not.

Yeah - we run both the pull request and the master + branch merge every time.

Re: A better pull request

#33
Solution: Don't ever merge branches using a web GUI. Do the merge locally and then push to master.

For the past couple of years I've only used GitHub's web-based PR tool for code discussion/peer review. Don't ever click that "merge" button.

Another reason I hate the merge button: It creates an extra commit solely for the merge.

Re: A better pull request

#34
post #25

Earlier quoted context omitted.

It speaks to github's prominence in the open source community and Atlassians's prominence in professional/enterprise environments.

Are there that many non-enterprise companies using Bitbucket over Github? Most startups and mid-sized companies I know use Github for private repo hosting.

I think the integration with JIRA (which is arguably best-in-breed for moderately heavy-duty issue tracking) is quite a compelling argument for going with an all-Atlassian setup; you can even use their SourceTree product as a git GUI.

On the other hand, GitHub's stuff generally does feel nicer to use and better and more thoughtfully UX'd.

Re: A better pull request

#35

Solution: Don't ever merge branches using a web GUI. Do the merge locally and then push to master. For the past couple of years I've only used GitHub's web-based PR tool for code discussion/peer review. Don't ever click that "merge" button. Another reason I hate the merge button: It creates an extra commit solely for the merge.

Still, TFA raises a good point, which is that the diff should be between the feature branch and current master, not master at the time the branch was created.

Re: A better pull request

#36
post #14

I agree this is a better way to look at pull requests, but I'm surprised it's as much of a problem as the post implies. For us, whenever we do a pull request, we always rebase against the latest master first. That can be a cat and mouse game, but it usually isn't. Our repos don't change that rapidly.

The problem is, many pull requests take very long to get merged (because of the size of the PR, the review process, etc.), and by the time that they are ready to be merged, the target branch could very well have changed.

This is something that our team has relegated to a product development problem not a software development problem. As a product manager/technical lead you have to realize it's your job to make it as easy as possible for developers to do their job. When you set them up for HUGE merge conflicts - that only makes life harder. If you're really good about constructing discrete, small stories, you can avoid SO many pull request headaches. If you don't allow master to get miles ahead by merging small changes frequently, this is a non-issue. It's hard to do though and requires changing the mindset of your team.

Re: A better pull request

#37
post #14

I agree this is a better way to look at pull requests, but I'm surprised it's as much of a problem as the post implies. For us, whenever we do a pull request, we always rebase against the latest master first. That can be a cat and mouse game, but it usually isn't. Our repos don't change that rapidly.

Talking to a lot of people at conferences and similar events, rebasing against the target branch before merging is pretty uncommon. Few people think a clean history is that important.

Personally I think, rebase + auto-squash/auto-fixit makes the history a lot easier when it's time to look back. It just happens so rarely I wonder if it's really worth the effort I expend on it.

Re: A better pull request

#38
post #14

I agree this is a better way to look at pull requests, but I'm surprised it's as much of a problem as the post implies. For us, whenever we do a pull request, we always rebase against the latest master first. That can be a cat and mouse game, but it usually isn't. Our repos don't change that rapidly.

The problem is, many pull requests take very long to get merged (because of the size of the PR, the review process, etc.), and by the time that they are ready to be merged, the target branch could very well have changed.

Our solution to that is to always rebase & push right before you run through ci & merge. As well as updating the PR diff to be against the master it was actually merged against, this also has the advantage of making your git history a lot more linear[0], which has various other upsides (e.g. makes it a lot more sane when using fugitive or something to navigate backwards & forwards through the history of a file).

[0] i.e. looks like http://blog.carbonfive.com/wp-content/uploads/2010/12/multip...

Re: A better pull request

#39
post #7

Seems quite sensible. This, some commentary on forking[1] I ran across the other day, and the way SourceTree is miles beyond github's client, all make me suspect I should be at least trying out bitbucket. But since literally everything I collaborate with is on github, I've not gotten around to it. Am I missing out, or does bitbucket have its own weak spots? [1] http://zbowling.github.io/blog/2011/11/25/github/

I use bitbucket mainly to host all my private repos (unlimited free private repos) and use github for my public ones. I'm not sure what the tradeoffs are regarding features (so far I'm not missing anything on bitbucket) but it's still git so the core things are still the same.

I'd argue bitbucket has more features. At least in terms of granular repo permissions, they are way ahead.

Re: A better pull request

#40

Earlier quoted context omitted.

On GitHub you can have tests run before you've merged, so you know whether it's safe or not.

Can't you run unit tests before you merge on an Atlassian service too?

You can, though you doing get to do it by using the ref/pull/## trick. That's just a connivence, you can get CI to do about anything.
Post reply on HN