A better pull request(developer.atlassian.com)
developer.atlassian.com
A better pull request
https://developer.atlassian.com/blog/2015/01/a-better-pull-request/
24 comments
That does not solve what I consider the biggest problem, the clean merge that actually results in a logical bug. Without being able to review the diff between the tip of master that will happen with your solution regardless.
EDIT: currently I always rebase our feature branches before submitting the PR to try and mitigate this and make sure review happens quickly after that. it doesn't fully solve the problem but it's the best flow I've found.
EDIT: currently I always rebase our feature branches before submitting the PR to try and mitigate this and make sure review happens quickly after that. it doesn't fully solve the problem but it's the best flow I've found.
The merge example that introduced a logical bug in the article was only visible in the diff because the bug occurred within 3 lines of the added line in the feature branch.
The best way to avoid this issue is automated testing. The second best way is to crack open the file itself, and review the entirety of any functions that changed. Even that approach assumes your encapsulation is nice and you're not introducing issues based on global state though.
While the BitBucket diff is better than nothing (and better than GitHub/GitLab), it's not sufficient to avoid these kinds of issues entirely.
The best way to avoid this issue is automated testing. The second best way is to crack open the file itself, and review the entirety of any functions that changed. Even that approach assumes your encapsulation is nice and you're not introducing issues based on global state though.
While the BitBucket diff is better than nothing (and better than GitHub/GitLab), it's not sufficient to avoid these kinds of issues entirely.
> EDIT: currently I always rebase our feature branches before submitting the PR to try and mitigate this and make sure review happens quickly after that. it doesn't fully solve the problem but it's the best flow I've found.
I rebase after the PR has been discussed and "approved" for merging. I feel having the individual commits during discussion time are useful for context, so long as team members are earnest enough to use them. Usually good commit messages can answer every "Why did you do it this way?" question before it even gets asked.
I rebase after the PR has been discussed and "approved" for merging. I feel having the individual commits during discussion time are useful for context, so long as team members are earnest enough to use them. Usually good commit messages can answer every "Why did you do it this way?" question before it even gets asked.
Why would you not be able to review the diff with a local tree? If anything that's much easier in a local environment.
The GUIs by nature hide detail: the pull request becomes a thing to be "displayed" instead of "explored". It's a problem that requires careful attention to detail. I don't know that I actually disagree with Atlassian's decision, but the fact that it had to be made isn't evidence that it's the "right" solution either.
The GUIs by nature hide detail: the pull request becomes a thing to be "displayed" instead of "explored". It's a problem that requires careful attention to detail. I don't know that I actually disagree with Atlassian's decision, but the fact that it had to be made isn't evidence that it's the "right" solution either.
> the clean merge that actually results in a logical bug
Theoretically possible, Probability < 3%. Besides, if master has tests and your branch has a test for the branch's feature, and you still have this problem, then maybe the 2 developers are overlapping so much that they should pair.
Theoretically possible, Probability < 3%. Besides, if master has tests and your branch has a test for the branch's feature, and you still have this problem, then maybe the 2 developers are overlapping so much that they should pair.
Wow, the first thing I do when creating a new repo is disable fast-forwarding, especially for large teams.
The merge commit is the quickest way to see all the changes that came in from a branch, and if you do branching right, all the changes related to a particular feature by one developer.
Also, merge commits are much easier to roll back, no matter how rarely you need it.
The merge commit is the quickest way to see all the changes that came in from a branch, and if you do branching right, all the changes related to a particular feature by one developer.
Also, merge commits are much easier to roll back, no matter how rarely you need it.
Another option, used by large teams at Facebook, is to only use fast-forwarding but squashes all branch commits. You can still roll-back a merge, but your master's revision history is still linear (for what that's worth).
This is how I prefer to handle PRs, too. `git merge --squash pr-branch`. Prevents the history from turning into an indecipherable tangle of branches, and prevents tons of "fixing typo," "code review feedback," "more code review feedback," "adding back file" kinds of commits from taking over the history.
We are on GH, it's quite annoying that they provide 0 support for rebase/squash style PR application. http://stackoverflow.com/q/27974175/1366219
We are on GH, it's quite annoying that they provide 0 support for rebase/squash style PR application. http://stackoverflow.com/q/27974175/1366219
You can rebase commits in your feature branch as well and clean up the history. Having separate and small commits also helps in blaming and looking at the history do figure out why a certain change is in there.
If you squash all the commits you probably should summarize all the commits into that single commit message which is also work if done properly.
In the end it's preference and how your developers create commits and documment them.
If you squash all the commits you probably should summarize all the commits into that single commit message which is also work if done properly.
In the end it's preference and how your developers create commits and documment them.
Yes this is what I advocate doing as well in my post above.
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.
I disagree. It should be both. Specifically because what was tested and run by the person sending in the request was the diff to the master at the time the branch was created.
Please let that sink in. Nobody ever tested the diff that this is going to be showing to the user.
Please let that sink in. Nobody ever tested the diff that this is going to be showing to the user.
But once you click the "Merge" button in GitHub, nobody has ever seen or tested the new state of master.
I'd rather see the diff that's going to go into master (tested or not) than see a diff that effectively means nothing (as it will never be applied to anything).
I'd rather see the diff that's going to go into master (tested or not) than see a diff that effectively means nothing (as it will never be applied to anything).
That is not necessarily true. The test results (e.g. travis, but you can use for instance Jenkins for your Github repository) shown next to the merge button are the result of a merge between the pull request branch and the branch you want to merge into. That's why when you do an ls-remote, you will see something like:
c3ca093c99495d0ddbb3197c14e0cdf514266392 refs/pull/2/head 0f5993828108bd2960713dc34a9ac7bef6dbc653 refs/pull/2/merge
one is the head of the pull request, the other is the result of the merge that you will end up with. Though I'm not sure Github will trigger another build when the target branch moves.
c3ca093c99495d0ddbb3197c14e0cdf514266392 refs/pull/2/head 0f5993828108bd2960713dc34a9ac7bef6dbc653 refs/pull/2/merge
one is the head of the pull request, the other is the result of the merge that you will end up with. Though I'm not sure Github will trigger another build when the target branch moves.
That's how we build using Jenkins - it's a nice feature, but it doesn't retrigger when the target branch changes. So, while it's useful so far as that it's better than simply building the branch, it doesn't ensure that "what you tested is what you get."
In the Rust project, the merging itself is automated and the merge bot (bors) runs the test after having effected the merge. The merging is attempted after the branch has passed review.
That doesn't preclude running e.g. travis to get the branch's own status before the review happens.
That doesn't preclude running e.g. travis to get the branch's own status before the review happens.
This is an argument against github and bitbucket style pull requests, though. And is basically my point.
If you use the Travis CI integration, it actually tests the result of merging a PR, rather than the branch being PRed. This seems to solve a lot of problems on projects with thorough testing.
Yes, but this is the diff that is going to happen if it's merged, so it's what reviewers should be looking at.
I disagree still. Fundamentally, I agree that it is important. I'd go further and say that these two diffs should be similar. If they are dissimilar, then there should be a red flag.
That's a good point. The diffs would be more useful both the feature branch's changes (Bob's changes in the article's example) and master's diverged changes were highlighted (in different colors):
https://bitbucket.org/tpettersen/abpr/pull-request/2/fixed-a...
https://bitbucket.org/tpettersen/abpr/pull-request/2/fixed-a...
Yea same here. I stopped using github's PR for similar reasons since I always rebase onto master locally and compare that diff. Seems like the bitbucket feature is emulating this which sounds like a great improvement to me.
Something doesn't add up for me. I constantly push the github "merge" button, and I've never, ever, seen an error AFTER I push it. It it wasn't checking for mergeability on actually current master, that seems surprising.
Is the issue that the button knows if a pull will merge ok, because it does test on current master, but the diff shown in the UI is different from that, and not on current master, and just on the older commit? If so, I think this could have been clarified better in the post.
Even so, this means that sometimes the button should be red - can't merge, has conflicts - while the diff as shown looks fine. I've never seen that happen. So something is puzzling here.
Is the issue that the button knows if a pull will merge ok, because it does test on current master, but the diff shown in the UI is different from that, and not on current master, and just on the older commit? If so, I think this could have been clarified better in the post.
Even so, this means that sometimes the button should be red - can't merge, has conflicts - while the diff as shown looks fine. I've never seen that happen. So something is puzzling here.
Github does check if it's able to merge to the head of the intended branch automatically, and enable or disable the merge button.
But the point of the article is that the context shown in the files (diff) view on github is that of the common ancestor, and doesn't include later changes to the branch being merged into. In addition to not showing merge conflicts if they exist (even though it disables the merge button), it also hides logic conflicts. These are situations where a clean automatic merge is possible, but the result will be incorrect logic due to the slightly distant interaction of those not-shown later changes. The simplest example of this was in the blog post.
All that said, even the bitbucket diff view doesn't show you logic conflicts caused by changes in completely different files, or otherwise more-distant. Nothing really does, it would be too much.
So, in an ideal world, you thoroughly test the merged result (in addition to the feature branch itself) before actually merging into the upstream branch.
But the point of the article is that the context shown in the files (diff) view on github is that of the common ancestor, and doesn't include later changes to the branch being merged into. In addition to not showing merge conflicts if they exist (even though it disables the merge button), it also hides logic conflicts. These are situations where a clean automatic merge is possible, but the result will be incorrect logic due to the slightly distant interaction of those not-shown later changes. The simplest example of this was in the blog post.
All that said, even the bitbucket diff view doesn't show you logic conflicts caused by changes in completely different files, or otherwise more-distant. Nothing really does, it would be too much.
So, in an ideal world, you thoroughly test the merged result (in addition to the feature branch itself) before actually merging into the upstream branch.
I think I get it now, thanks. My confusion was I didn't realize that the github merge button is "smarter" and more recent than the diff being show. I always assumed they were in sync, which is definitely surprising.
From my experience, Github does attempt a merge commit behind the scenes and indicates that a PR can't be merged via the UI if there are conflicts. However, it still just shows the diff from the merge-base to the tip of your feature branch, which won't include any changes that have been added to master since the branch point.
I see. So the button status is more up to date than the diff, and I suppose it is rare enough that the two are out of sync, that people just don't see it much? Makes sense I guess.
The merge button status is always in sync with the diff. It is disabled if the diff won't merge correctly.
To answer your earlier question more directly: the diff that is shown is a diff against an earlier master. (It's in fact a diff against the point where the branch forked off of master.)
To answer your earlier question more directly: the diff that is shown is a diff against an earlier master. (It's in fact a diff against the point where the branch forked off of master.)
As others have said, GitHub does produce a merge commit for the PR. You can checkout that commit like so,
git fetch origin +refs/pull/$N/merge && git checkout FETCH_HEAD
However, in my experience I have seen both cases where GitHub makes new merge commits when the receiving branch changes (current master) and where it does not. Due to externalities in our test suite we sometimes have "current master" break and when we push a fix we sometimes see PRs go green and sometimes they do not. I haven't figured out why we see the two different behaviors.
git fetch origin +refs/pull/$N/merge && git checkout FETCH_HEAD
However, in my experience I have seen both cases where GitHub makes new merge commits when the receiving branch changes (current master) and where it does not. Due to externalities in our test suite we sometimes have "current master" break and when we push a fix we sometimes see PRs go green and sometimes they do not. I haven't figured out why we see the two different behaviors.
GitLab B.V. CEO here. The proposed diff is interesting and I love the way they explained the idea. During the development of GitLab (over 10.000 commits) we never experienced the problem that was mentioned. Maybe it is because our codebase contains many tests. One related idea that a GitLab user proposed is not testing the feature branch but testing the merge of the feature branch into master. This would require retesting of all feature branches on every commit to master. We're open to proposals to add this to GitLab CI. But so far we didn't experience this problem frequently.
The idea of testing the merge commit is a great one. Even if the test suite is able to catch the 'logical conflict' mentioned in the article, current solutions will only catch the conflict after it's merged into master.
Of course, it's not too hard for the person responsible for merging the pull request to replicate this themselves (branch off master, merge in feature branch, run tests).
Of course, it's not too hard for the person responsible for merging the pull request to replicate this themselves (branch off master, merge in feature branch, run tests).
Thanks! Indeed you can replicate it yourself but we like to automate as much as possible.
I hear GitLab EE has a 'rebase before merge' checkbox next to the 'merge' button. What I'd like to see is have it enforced, that is, the UI won't allow a merge if the MR has not been rebased onto master's tip. Enforcing this policy has saved us a few headaches, including the article's case which happened tree times on a project counting ~2000 commits.
(BTW, we love GitLab, congrats on the new UI layout, which has rave reviews coming back to me mere hours after the upgrade)
(BTW, we love GitLab, congrats on the new UI layout, which has rave reviews coming back to me mere hours after the upgrade)
It is exactly what Travis CI does for auto-PR testing on Github: they pull in master (or whatever base) and fetch the feature branch. They create the commit themselves and test the result. That way, you always get the tests based on the merged result which gives far more insights than tests on your (local) feature branch.
Very interesting, I never considered looking at the conflicts that way. It might be helpful to allow looking at the conflicts in the diff3 conflict marker style. The advantage is that you get to see the common ancestor, which can be crucial to figuring out a tricky merge.
git config --global merge.conflictstyle diff3
This is so telling: GitHub had a blog post yesterday[1] about pull requests and now Atlassian does today with a very similar title. The one from GitHub was about social dynamics and how to work together better, while the one from Atlassian is about technical minutiae.
Having used both GitHub and Stash, the difference in focus between the two companies comes across plainly, and these two blog posts only back it up.
[1]: https://github.com/blog/1943-how-to-write-the-perfect-pull-r...
Having used both GitHub and Stash, the difference in focus between the two companies comes across plainly, and these two blog posts only back it up.
[1]: https://github.com/blog/1943-how-to-write-the-perfect-pull-r...
Atlassian are talking about a real problem here, which problem they appear to have solved. Working code is usually superior to social convention (rebase, etc.); that's why we're using git (or possibly other DVCSs) instead of RCS, etc.
Not sure I agree. In either case, both a social convention and an expected response from the executed command are necessary because both the "double dot" and "triple dot" logic are necessary to truly understand the implications of a complex pull request.
In the case of Atlassian's solution, the "merged" social convention will have to be use to first use and personally verify the "triple dot" findings in order to review that the change to the branch is, in isolation, as expected - before proceeding through the "double dot" flow that now characterizes the pull request feature.
For Github, it's simply the inverse.
Either way, both are relatively easy to achieve with the command line or a diff tool like Meld or LiClipse.
In the case of Atlassian's solution, the "merged" social convention will have to be use to first use and personally verify the "triple dot" findings in order to review that the change to the branch is, in isolation, as expected - before proceeding through the "double dot" flow that now characterizes the pull request feature.
For Github, it's simply the inverse.
Either way, both are relatively easy to achieve with the command line or a diff tool like Meld or LiClipse.
It speaks to github's prominence in the open source community and Atlassians's prominence in professional/enterprise environments.
Personal use = BitBucket
WHY? Free private repos unlimited in number of repos only limited on how many can access them.
So all my .config files and anything private goes to BitBucket. Great product for my own use.
WHY? Free private repos unlimited in number of repos only limited on how many can access them.
So all my .config files and anything private goes to BitBucket. Great product for my own use.
If it’s personal stuff, why do you need an upstream?
Assuming it’s for redundancy, why not just copy the folders as part of a normal backup?
Assuming it’s for redundancy, why not just copy the folders as part of a normal backup?
Having an upstream can be useful for CI testing and for deployments with tools like Capistrano. Plus it allows you to pull down your code from other environments if needed and acts as a additional backup, so there are potentially lots of reasons to have one.
Because even though it's personal stuff, it's shared between many computers. For people that live in VCSs all day, it's just more convenient (history, branches, etc.) than e.g. storing it on Dropbox or similar.
Cloud access? I have a similar problem where my various machines are behind NATs/firewall (personal computer, workstation...). It could just be more convenient to put it in the cloud rather than have to poke port forwarding or VPN rules between the machines.
Well, there's the obvious benefit of tracking changes and it also provides a nice way to sync files selectively between computers.
In my case, OS X has the latest git, while Windows is stuck on 1.9, and my git folders get corrupted if I do a push on OS X and simply do a status on Windows. I guess something has changed between 1.x and 2.x but this corruption is really not something which should ever happen in a VCS. My solution is to use Gitlab instead of Dropbox only local repos.
I'm not aware of any changes between 1.x and 2.x that would corrupt a repo... I use a mix on various systems, some even still have git 1.7.x on them. The only problem I run into is when you have some script that depends on a newer git feature added in a newer version...
My money would be on Dropbox busting your repo somehow. I don't think Dropbox is an ideal solution for pushing/pulling git repos from.
My money would be on Dropbox busting your repo somehow. I don't think Dropbox is an ideal solution for pushing/pulling git repos from.
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.
On the other hand, GitHub's stuff generally does feel nicer to use and better and more thoughtfully UX'd.
Side note, but you can use SourceTree regardless, right?
I assume it has some features that are paid (or only work with Bitbucket?), but I've found it brilliant for managing GH repos - one of the nicest free tools I use.
I assume it has some features that are paid (or only work with Bitbucket?), but I've found it brilliant for managing GH repos - one of the nicest free tools I use.
Nope, SourceTree is now 100% gratis. You still have to "register" it and apply a key after 30 days, but this involves a simple signup form on a website.
Bitbucket provides free private repos, and for smaller development teams and/or companies not wanting to deal with the cost of Github (which does admittedly grow exponentially the more repos you require), Bitbucket is a fine choice.
When we were making the decision at my company, we went with Github because the dev team cared about having the little green squares show up on the "activity" chart for their account's... I know, petty, but it's something, and since most of us do FOSS projects, it's a status thing.
It used to be any commit that made it into a repo's master branch, the green squares showed up, even if it was a private repo (it just didn't show details of the repo to public users). But now, those don't show up to the public, only the user themselves sees them while logged in... so if we were making the decision today, I'd probably lean towards Bitbucket.
When we were making the decision at my company, we went with Github because the dev team cared about having the little green squares show up on the "activity" chart for their account's... I know, petty, but it's something, and since most of us do FOSS projects, it's a status thing.
It used to be any commit that made it into a repo's master branch, the green squares showed up, even if it was a private repo (it just didn't show details of the repo to public users). But now, those don't show up to the public, only the user themselves sees them while logged in... so if we were making the decision today, I'd probably lean towards Bitbucket.
which does admittedly grow exponentially the more repos you require
Just to clarify, GH's pricing doesn't actually grow exponentially. The per repo price gets lower the more you pay for:
5 private repos: $7
10 private repos: $12
20 private repos: $22
50 private repos: $50
Just to clarify, GH's pricing doesn't actually grow exponentially. The per repo price gets lower the more you pay for:
5 private repos: $7
10 private repos: $12
20 private repos: $22
50 private repos: $50
For a team, pricing is different. 50 repos is $100 a month, 125 repos is $200 a month ($2,400 a year). Granted, it's not "exponential", I was using a figure-of-speech referring to the pricing becoming significant.
For an internal-dev team which generates a great deal of new repos throughout the year (one-off scripts/programs for different departments, etc...), this adds up very quickly.
If you reach that 126th repo, it jumps to $450 monthly or $5,400 a year. At those prices you get questions from Accounting about why we aren't hosting this internally...
For an internal-dev team which generates a great deal of new repos throughout the year (one-off scripts/programs for different departments, etc...), this adds up very quickly.
If you reach that 126th repo, it jumps to $450 monthly or $5,400 a year. At those prices you get questions from Accounting about why we aren't hosting this internally...
GitHub Enterprise is an appliance VM you run internally. It's charged per seat. The last enterprise-y company I was at used it primarily because you keep your code inside the firewall, but also because the pricing model is more aligned with the usage in that environment, as you suggest.
GitHub Enterprise is even more expensive than what I quoted above. The quotes above are for private "team" repos under an "Organization" on Github (they host it all).
GitHub Enterprise is pretty pricey. It's almost exclusively about being able to host internally.
It can depend highly on your needs. We have a small team but a large number of private repositories. Github basis prices on number of repositories, which makes them incredibly expensive for us. Bitbucket basis prices on team size, which made them cheap for us.
We use BitBucket since we're a small shop from a developer count standpoint but we have a ton of small repos plus we migrated over a ton of legacy svn repos. GitHub gets expensive real quick when you are in that situation.
For anyone doing 'client work' rather than developing a single/few products, GitHub's pricing is almost prohibitively expensive.
We have a little over a hundred repositories. This puts is in the $200/mo plan for GitHub (125 repository limit).
Atlassian prices per-user. Our small three person dev team costs us nothing. We'll reach the next pricing tier when we hit our 6th developer, at which point it will cost us $10/mo and will remain that cost up to 10 developers.
BitBucket's top plan is $200/mo. That gets you unlimited repositories and users compared to GitHub's 125 repositories.
In the middle tier, GitHub charges $50/mo for 20 repositories whereas BitBucket charges $50/mo for 50 users.
If you have few repositories but many users, GitHub's pricing is advantageous. If you have many repositories but few users, BitBucket makes way more sense.
We have a little over a hundred repositories. This puts is in the $200/mo plan for GitHub (125 repository limit).
Atlassian prices per-user. Our small three person dev team costs us nothing. We'll reach the next pricing tier when we hit our 6th developer, at which point it will cost us $10/mo and will remain that cost up to 10 developers.
BitBucket's top plan is $200/mo. That gets you unlimited repositories and users compared to GitHub's 125 repositories.
In the middle tier, GitHub charges $50/mo for 20 repositories whereas BitBucket charges $50/mo for 50 users.
If you have few repositories but many users, GitHub's pricing is advantageous. If you have many repositories but few users, BitBucket makes way more sense.
Tons of startups that you don't even know about use Bitbucket for their free private repos. When you have a team of 1-5 people there is no reason to pay for Github.
Bitbucket also supports Mercurial, which I find to be more user-friendly than Git with all of the same features.
You'd think more start-ups/mid-size ones would use Bitbucket since they have free private repos. I'm guessing the social allure of Github and its superior repo/pull request UI is what trumps Bitbucket to that end.
I'm pretty sure it's wanting to be one of the 'cool kids' that pulls people to GitHub.
If your decision about where to host your code repos is decided by how pretty the webpage to create pull-requests is, the wrong people are making decisions such in your business.
If your decision about where to host your code repos is decided by how pretty the webpage to create pull-requests is, the wrong people are making decisions such in your business.
The differences aren't as superficial as you suggest, in my experience. GitHub is just far more polished. Take commit history as an example: bitbucket makes it uncommonly awkward to step through a series of commit diffs for a given file, whilst github makes it a bit easier (although still not perfect - am I the only one who wants this feature?). Nicer looking doesn't always mean more usable, but there's often a correlation and, in this case, it definitely bears out.
Having said all that, I'm currently using bitbucket for a private work repo because I'm cheap :)
Having said all that, I'm currently using bitbucket for a private work repo because I'm cheap :)
Unsurprisingly, this is very much a personal choice. I vastly prefer BitBucket's issue tracker. I love their side-by-side diff (somewhat recently added to GitHub). I love that they don't try to force this 72 char commit message on me and seem to handle it rather nicely via hover text. And I very much liked that they didn't support emoji and inline GIFs (since implemented, lamentably).
The more GitHub seems to adjust itself, the less I enjoy it. The more BitBucket seems to try to copy GitHub, the less I enjoy it. At the end of the day, I probably want GitHub circa 2008. I find when they really became opinionated about things to be the inflection point about whether they pushed things that were truly more usable.
That's not to say your view of things is wrong for you. But I don't think it's clear that "in this case, it definitely bears out".
The more GitHub seems to adjust itself, the less I enjoy it. The more BitBucket seems to try to copy GitHub, the less I enjoy it. At the end of the day, I probably want GitHub circa 2008. I find when they really became opinionated about things to be the inflection point about whether they pushed things that were truly more usable.
That's not to say your view of things is wrong for you. But I don't think it's clear that "in this case, it definitely bears out".
> bitbucket makes it uncommonly awkward to step through a series of commit diffs for a given file, whilst github makes it a bit easier (although still not perfect - am I the only one who wants this feature?)
I want it too :) A time-machine style forward & back, showing changes to a file.
I want it too :) A time-machine style forward & back, showing changes to a file.
> decided by how pretty the webpage to create pull-requests is
i.e. whether the gui is well designed.
i.e. whether the gui is well designed.
> The one from GitHub was about social dynamics and how to work together better, while the one from Atlassian is about technical minutiae.
What is it telling us of? Are you implying something to the effect of Atlassian doesn't care about people working together (heck, maybe they hate humans, who knows) and Github loves community...?
What is it telling us of? Are you implying something to the effect of Atlassian doesn't care about people working together (heck, maybe they hate humans, who knows) and Github loves community...?
When I first encountered this behavior on BitBucket, I spent a lot of time reading the man pages and searching the internet trying to find out which option were they providing to `git diff` to get this diff output but couldn't find anything. Looks like there is indeed no such option. It would be awesome if git had something like `git diff branch-name --merge-commit-diff`.
One of the Bitbucket engineers put a one-liner in the comments on the post that will generate diff-like output similar to what you're describing: https://developer.atlassian.com/blog/2015/01/a-better-pull-r...
Yup, that was me who asked the question there. :-)
I think both the old and new diff worth a look. Maybe showing them using different colors is a good idea.
Phabricator uses light colors to show changes not introduced in this patch but a rebase against master [1]. The experience is pretty good if the workflow is to always fast-forward and the patch author does rebase. Changes introduced by other people are in light green and red. Duplicated-line issue can be obviously seen.
[1]: https://secure.phabricator.com/book/phabricator/article/diff...
Phabricator uses light colors to show changes not introduced in this patch but a rebase against master [1]. The experience is pretty good if the workflow is to always fast-forward and the patch author does rebase. Changes introduced by other people are in light green and red. Duplicated-line issue can be obviously seen.
[1]: https://secure.phabricator.com/book/phabricator/article/diff...
Wouldn't this result in an ever-growing diff? You'd see everything that's gone into master since the branch last rebased/merged master as "changed". When I think about a branch, I want to see changes unique to it, not what's going on in the rest of the codebase.
Showing merge conflicts inline like that is pretty cool, though.
Showing merge conflicts inline like that is pretty cool, though.
In Bitbucket's implementation you still only see the changes that are unique to your branch, but they're diff'd against the current tip of master rather than the merge base.
I've said this in another thread, but I feel it really needs repeating.
The diff that bitbucket is showing you is one that has not been tested. That is, the diff against the merge base shows what the requester did and tested. The diff against the current tip, shows what will be the result.
I fully agree that it is important to bear that in mind. But, this is all the more reason for the merge to be done by another party before committing/pushing.
Consider, if you send a pull request for the kernel, Linus will do a local merge, then test, then push. In all of these gui apps, the final merged code is just there. No last second "fails sanity test, so not really going to merge."
The diff that bitbucket is showing you is one that has not been tested. That is, the diff against the merge base shows what the requester did and tested. The diff against the current tip, shows what will be the result.
I fully agree that it is important to bear that in mind. But, this is all the more reason for the merge to be done by another party before committing/pushing.
Consider, if you send a pull request for the kernel, Linus will do a local merge, then test, then push. In all of these gui apps, the final merged code is just there. No last second "fails sanity test, so not really going to merge."
However, if per convention you state that a branch going under review must always be rebased to the latest commit of [your main branch], you will be reviewing all at once the code that will effectively be merged (by a simple fast-forward), and the code that the requester tested.
It seems to me that this is the safest method as, when you add an automated testing tool to the mix, it's pretty much guaranteed that you cannot break the main branch when merging a PR.
I think rebase is not advertised enough, but it's the _de facto_ solution to most of these kinds of problems.
It seems to me that this is the safest method as, when you add an automated testing tool to the mix, it's pretty much guaranteed that you cannot break the main branch when merging a PR.
I think rebase is not advertised enough, but it's the _de facto_ solution to most of these kinds of problems.
Close. Under this convention, somebody has to do a final test before pushing the merge commit. It can be a simple sanity test, not a full blown integration test. But somewhere somebody at least did a compile check.
If it fails that test, then you push it back to the person doing the work saying so and they need to fix it.
If it fails that test, then you push it back to the person doing the work saying so and they need to fix it.
In a GitHub project I've contributed to (dolphin-emu), a compile check for all supported platforms, plus a few tests, shows up next to the big merge button. So the "other party" doing the merge is a robot, and you can still click the button.
I haven't been a committer on any other large GitHub projects, so I'm not sure how common this is.
I haven't been a committer on any other large GitHub projects, so I'm not sure how common this is.
So, sadly, I don't contribute to really any github projects. This is a new feature to me and does go a long way to addressing my concern. Honestly, it may completely address it. Certainly sounds like it does.
True, but at least it shows you the actual result of applying the diff to the HEAD, as opposed to all other GUI tools that show you a diff that nobody has seen, let alone tested, in a merged state. So what is done by bitbucket is better in that regard.
That makes sense. Thanks for clarifying.
I'm surprised anyone would be willing to consider merging a branch into master that has not itself pulled the latest head from master. It puts the the person running the merge into the position of having to test someone else's code.
It would seem to me that if a branch diffs against an older point in master, then the PR should be rejected as not properly tested. Merging into master should never create a merge conflict; the merge conflict should happen and be resolved on the feature branch first.
Or what am I missing?
It would seem to me that if a branch diffs against an older point in master, then the PR should be rejected as not properly tested. Merging into master should never create a merge conflict; the merge conflict should happen and be resolved on the feature branch first.
Or what am I missing?
Or what am I missing?
The linux kernel averages >5 patches per hour. Even with multi-teired maintanership, good luck getting everyone to base their patch sets on the latest upstream head.
I know nobody other than the kernel actually uses git as a distributed system, but there are use cases where your ideal workflow can't actually exist.
The linux kernel averages >5 patches per hour. Even with multi-teired maintanership, good luck getting everyone to base their patch sets on the latest upstream head.
I know nobody other than the kernel actually uses git as a distributed system, but there are use cases where your ideal workflow can't actually exist.
This is tricky to pull off on busy projects, as master is continually progressing and all open pull request branches would need to be constantly rebased.
With the Bitbucket approach, you get to see the conflict in the pull request UI, but you should still resolve it locally on your branch before merging to master.
With the Bitbucket approach, you get to see the conflict in the pull request UI, but you should still resolve it locally on your branch before merging to master.
Ok this is nice. Of course, if you're just rebasing before reviewing and merging the PR into master it makes this pointless.
Odd that they never mention this in the post, I was under the assumption that rebasing before creating the PR is standard practice, at least that's how we do it at work and on all the projects I've contributed to.
EDIT: I've never used bitbucket, so maybe this is just a github community thing with the rebasing?
Odd that they never mention this in the post, I was under the assumption that rebasing before creating the PR is standard practice, at least that's how we do it at work and on all the projects I've contributed to.
EDIT: I've never used bitbucket, so maybe this is just a github community thing with the rebasing?
For smaller projects that's fine, but if you have a lot of outstanding pull requests under discussion that's a lot of continual rebasing as they each get merged. (What if the two fee fixes in the article were done at the same time and reviewed by different project maintainers?)
That sounds really unorganized to me at least, I mean the linux kernel is large and there's only one guy at the end of the day merging all that stuff.
I would think that in larger projects a better practice would be a distributed hierarchy of merges until a PR finally got submitted to master, not a massive blob of just send everything to master as a PR and let various maintainers merge whatever at will.
I would think that in larger projects a better practice would be a distributed hierarchy of merges until a PR finally got submitted to master, not a massive blob of just send everything to master as a PR and let various maintainers merge whatever at will.
That's a good (and standard) practice that solves the problem if your project never has more than one PR open at a time.
Something that bugs me with almost every source control GUI is that there is usually no way to spawn a three way diff after a merge has been done. They might support three way merge on conflict but bang, once you save you can not get that view back. They also often don't understand that no branch was the master branch so if you diff the merged revision (diffing a single revision doesn't make sense yes but guis usually present it this way and diff it with the last revision) then you get all changes that has happened on master since the branch was created, wtf?? To see what only the branch contributed you have to manually select two revisions, diff them and then select the other revision and diff.
Applying the God principle to this, there should never be merge conflicts. I should get notified that the file has changed before I even change it. That is how I imagine it would be if a God made it.
But at the time you modify it, the other file may not be modified. And, when your merge partner modifies it, she may be on an 18 flight to the South Pole with no internet access.
Now, sure, you can say "well, God can get around that", but, unfortunately, there are real constraints that cannot be gotten around. Good design is how you deal with those constraints.
Now, sure, you can say "well, God can get around that", but, unfortunately, there are real constraints that cannot be gotten around. Good design is how you deal with those constraints.
[deleted]
Why hasn't the alice/master branch brought over all of the changes in master since branching, and tested them, before review and (eventual) merge?
Forgive my ignorance but wouldn't rebasing your current branch from master before opening a pull request solve this issue?
That would solve the issue until master progresses again. The problem is that on many projects the master branch will progress during the lifetime of a pull request.
Surely the bug in logic would be caught by tests?
I can't really say why, but many things I've read from Atlassian/Bitbucket in recent months always had the feeling of B class (in contrast to A class). Even at the end of the article I still don't get the real problem that is solved. Commiting Mergeconflicts is not an advantage. And that logical error thingy I have never heard or seen before. Maybe because the people (who's opinion I care about) review commit diffs in detail and are able to rebase -i anything that looks bad?
Atlassian's bug tracker (Jira), wiki (Confluence) and CI system (Bamboo) all feel that way to me; B team, or even C. "Atlassian--we make software that you can eventually get to work in some fashion but will never please you in any way."
Jira is almost OK, but still has that enterprise stank of trying to do too much and inserting itself too much in your workflow, instead of helping you and getting out of your way. Some people are less generous toward Jira than I am: "Next time I'm looking for a job I think use of @jira might be a deal breaker."--Peter Seibel[1].
I have no generosity toward Confluence, their wiki. Bad UI choices, a weird & inconsistent markup language that again becomes another thing to wrestle with indefinitely instead of unobtrusively helping you. I've had to use it at two jobs, and there won't be a third.
Bamboo, their CI system, seems OK, but for some reason I end up debugging a lot of build failures having to do with leftover or broken state on the build machine, which seems like one of the first things a CI system should help you avoid.
[1]https://twitter.com/peterseibel/status/451758184470835200
Jira is almost OK, but still has that enterprise stank of trying to do too much and inserting itself too much in your workflow, instead of helping you and getting out of your way. Some people are less generous toward Jira than I am: "Next time I'm looking for a job I think use of @jira might be a deal breaker."--Peter Seibel[1].
I have no generosity toward Confluence, their wiki. Bad UI choices, a weird & inconsistent markup language that again becomes another thing to wrestle with indefinitely instead of unobtrusively helping you. I've had to use it at two jobs, and there won't be a third.
Bamboo, their CI system, seems OK, but for some reason I end up debugging a lot of build failures having to do with leftover or broken state on the build machine, which seems like one of the first things a CI system should help you avoid.
[1]https://twitter.com/peterseibel/status/451758184470835200
There are so many very small things. Like Bitbucket adds a web-editor like Github, but the editor automatically replaces all new lines with \r\n and has no field for a commit message (corresponding bug not treated until today and only considered minor priority, maybe because most people use Windows anyway or what). Or the markup language that has the ability to highlight code but only in 3 or so languages. You don't even have the choice to switch to others. The wiki I have only used two or three times. Can't get warm with it either.
It seems like the example of double fixing the calculation is a great reminder of the value of unit tests. Even if both commits added a unit test in slightly different ways so they didn't conflict, you'd just end up with a failing build and know something got screwed up.
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.
Don't the tests just run on the branch before the merge? If so, you wouldn't actually see the failure until after you merge.
> Don't the tests just run on the branch before the merge?
The merge button is only green if github could generate a merge commit, and that merge commit is made available. You can run your tests on either the branch before merge or the branch after merge, as you prefer.
You may want to do both and have the former block the latter: the merged head is going to change (and require a re-test) any time the target branch gets a new commit, no point in wasting cycle if the branch's own tests don't pass in the first place.
The merge button is only green if github could generate a merge commit, and that merge commit is made available. You can run your tests on either the branch before merge or the branch after merge, as you prefer.
You may want to do both and have the former block the latter: the merged head is going to change (and require a re-test) any time the target branch gets a new commit, no point in wasting cycle if the branch's own tests don't pass in the first place.
You can do either. Or both if you really want.
$ git ls-remote
From https://github.com/cyaninc/git-fat.git
4c86c39bb5fca55a692d11680ed62fd5cb183921 refs/pull/14/head
cb850f9d0bd09f8bb903e1c5959cef24ceb51695 refs/pull/14/merge
I'm guessing this is because a merge commit is created before the merge is fully executed.
[deleted]
Yeah - we run both the pull request and the master + branch merge every time.
Can't you run unit tests before you merge on an Atlassian service too?
I'm not sure about BitBucket as I don't use it, but I expect so.
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.
Fair point!
I really like the idea of merge conflicts being resolved in the open in a pull request, instead of the pull requester working in seclusion. For those of us with projects on GitHub, is there any way to replicate this behavior there?
A common practice is for pull requesters to rebase their branch on the upstream branch to avoid these conflicts.
But if the master/upstream is changed, the feature branch must be updated again (manually).
There is no magic to avoid this problem. If you are trying to get changes into a repository that is constantly changing with no good isolation of concerns, then expect pain.
Otherwise, merges really are better from most perspectives. I can trust people doing pull requests to have tested their code. Looking at the log, I can get an idea of whether or not their code was tested with someone elses. Something I can not do with a pure "rebase and push" mentality.
Otherwise, merges really are better from most perspectives. I can trust people doing pull requests to have tested their code. Looking at the log, I can get an idea of whether or not their code was tested with someone elses. Something I can not do with a pure "rebase and push" mentality.
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/
[1] http://zbowling.github.io/blog/2011/11/25/github/
I use both. I find Bitbucket just lacks polish— it's hard to come up with specific examples, but various things give off the vibe of it being maintained by a small team within a larger organization for whom it's not really a priority.
Code search is one, but I also find it harder to get to the exact diff or pull request that I want. I don't get Github's nice rendering of filetypes like STLs, and there's a lot less of the context-sensitive popups when you're mentioning users, issues, commits, etc.
Bitbucket's issue tracker is kind of limited from ever getting too good, because it's really just there as a teaser for JIRA.
Code search is one, but I also find it harder to get to the exact diff or pull request that I want. I don't get Github's nice rendering of filetypes like STLs, and there's a lot less of the context-sensitive popups when you're mentioning users, issues, commits, etc.
Bitbucket's issue tracker is kind of limited from ever getting too good, because it's really just there as a teaser for JIRA.
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.
Same, we use Stash locally due to latency, ($10/yr for our tiny team), then mirror to Bitbucket each commit automatically.
What about in-repo code search ?
https://bitbucket.org/site/master/issue/2874/ability-to-sear...
https://bitbucket.org/site/master/issue/2874/ability-to-sear...
I find that the search results from Github are often useless - sorting by relevance is hard in terms of code, I suppose.
Personally I enjoy Bitbucket for two reasons: a) free private repos mean I can use it as a remote backup for my projects, since I'm of the philosophy that you shouldn't be talking too much until you have at least a 0.1 version to show, b) no gamification and distractions.
The gamification and awkward "social coding" environment that GitHub provides just feels forced to me. It's distracting from your work and it makes you focus on pointless minutiae, but worse - it makes other people apply unnecessary weight on the significance of how "active" and "competitive" your profile is. Look at all the people who legitimately believe GitHub should be the be-all-end-all spot for software development, replacing your resume. Look at how Git the DVCS is becoming synonymous with GitHub the (centralized) platform.
With Bitbucket, I have none of that. I can focus on my project without any intervention from the peanut gallery, without all the pointless competition and addictive side jobs that GitHub conditions you into partaking in.
Also, I always found GitHub's issue tracker to be rather underdeveloped. I think it speaks about the "quick hack" culture of software development in general.
Oh, and I think Mercurial needs more attention. I use both hg and git.
The gamification and awkward "social coding" environment that GitHub provides just feels forced to me. It's distracting from your work and it makes you focus on pointless minutiae, but worse - it makes other people apply unnecessary weight on the significance of how "active" and "competitive" your profile is. Look at all the people who legitimately believe GitHub should be the be-all-end-all spot for software development, replacing your resume. Look at how Git the DVCS is becoming synonymous with GitHub the (centralized) platform.
With Bitbucket, I have none of that. I can focus on my project without any intervention from the peanut gallery, without all the pointless competition and addictive side jobs that GitHub conditions you into partaking in.
Also, I always found GitHub's issue tracker to be rather underdeveloped. I think it speaks about the "quick hack" culture of software development in general.
Oh, and I think Mercurial needs more attention. I use both hg and git.
I sometimes do quick patches of things that get in my way. Sometimes I do pull requests on github. Sometimes (usually?) I don't. And I use AWS, Google, Drop Box and Github to hold things for me in the face of (yet another) hard drive crash.
I like git well enough. Github seems nice. What is this gamification of which you speak? I'm not saying that it isn't staring me in the face, but I'm probably looking right past it.
I like git well enough. Github seems nice. What is this gamification of which you speak? I'm not saying that it isn't staring me in the face, but I'm probably looking right past it.
Yes! Mercurial love on HN always makes me happy. Code hosting platforms should encourage choice.
It would make me happy if SublimeGit was better at showing merge conflicts. I feel like I only know they exist when I'm surprised that my build is now broken, and my only resolution is to Find in Project for `>>>>>` and hope I catch all of them.
I've never used SublimeGit (or heard of it before today, to be honest), but if that's how you fix merge conflicts it sounds like a broken product.
git status will tell you which files are conflicted.
It'd make me happy if SublimeGit fixed the pull issues on OSX, let alone any update :(
Interestingly, only Github manages to syntax highlight the diff.
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.
Great point. Almost all of the problems I've seen with pull requests and branching workflows stem from the size of the change. If you can avoid long-lived branches / giant diffs, most integration problems just disappear. It's not always possible to avoid them, but bearing it in mind when you're constructing stories and planning your backlog helps a lot.
This is more of a problem in open-source projects where occasionally someone would drop by a project and submit a PR without any prior planning, and more often than not disappear for a while before coming back to address your comments/review. It is less of a problem in closely-tied teams.
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...
[0] i.e. looks like http://blog.carbonfive.com/wp-content/uploads/2010/12/multip...
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.
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.
YMMV. I've seen people all over the map on this.
I have noticed that among people I respect, there is a strong correlation between using git-bisect and wanting a clean history.
I have noticed that among people I respect, there is a strong correlation between using git-bisect and wanting a clean history.
I don't know what it is, but when I try to rebase master into the feature branch, my PR diff ends up littered with commits that aren't part of the PR. Then my reviewers have to wade through a bunch of irrelevant crap to see my changes. I thought the whole point of rebasing was so that wouldn't happen.
Anyway, now I just don't do it anymore. Git is a pain in the ass.
Anyway, now I just don't do it anymore. Git is a pain in the ass.
If you rebased master into the feature branch, then what you said makes sense. I realize it may look like semantics, but you rebase your feature branch onto the master branch. This means something completely different.
I am curious what you use as an alternative.
I am curious what you use as an alternative.
Huh. If I rebase master into the feature branch, isn't that supposed to move the point at which the feature branched off of master from where it was, to the HEAD of master?
About rebasing the feature branch onto master, my team doesn't do that, we squash the feature branch commits into one commit when merging to master.
(I don't use anything besides git, but that doesn't mean I can't hate it)
About rebasing the feature branch onto master, my team doesn't do that, we squash the feature branch commits into one commit when merging to master.
(I don't use anything besides git, but that doesn't mean I can't hate it)
No, if you rebase master into the feature branch, that will move all of the new commits to master into the feature branch.
Now, it could be that I am just being a stickler for phrasing here. So, to clarify, if you are on the branch and run 'git rebase master', that is not rebasing master into the feature branch. That is rebasing the feature branch onto master.
So, is that what you were doing?
Now, it could be that I am just being a stickler for phrasing here. So, to clarify, if you are on the branch and run 'git rebase master', that is not rebasing master into the feature branch. That is rebasing the feature branch onto master.
So, is that what you were doing?
My phraseology was wrong, apologies. Yes that is what I was doing. You can probably tell that my default attitude towards git is one of confusion and frustration, this is no different
No worries. I can not claim that it is a simple problem to just immediately understand. Worse, I am not good enough in my understanding, to explain in a message forum. :(
I can say that it is easy, once you understand it more. It will take some time.
Countering that point, though; if you have a codebase that is rapidly changing at all times... there really isn't anything git can do to help.
I can say that it is easy, once you understand it more. It will take some time.
Countering that point, though; if you have a codebase that is rapidly changing at all times... there really isn't anything git can do to help.
Well, the problem for me is what I perceive to be the discrepancy between how rebase is supposed to work, and what it actually does when I use it. I would like to think I understand it but I guess you could argue I don't. At any rate I'm not really learning about it, I just don't do it anymore because I kept getting burnt by it.
My teammates have suggested to simply merge master into the branch so I do that now. It adds a commit to the branch, but Github is smart enough not to litter up the PR diff with the merge commit.
Our codebase doesn't change that much, every commit to master has to go through a PR and get approved. So fortunately we don't have to contend with that.
My teammates have suggested to simply merge master into the branch so I do that now. It adds a commit to the branch, but Github is smart enough not to litter up the PR diff with the merge commit.
Our codebase doesn't change that much, every commit to master has to go through a PR and get approved. So fortunately we don't have to contend with that.
Only thing I can say is that rebase is exactly the same as reseting to the merge base, then cherry picking each individual commit that was lost in the reset. This means if you did 10 commits locally, it may stop 10 times to have you fix things. Versus a merge, which just does a single commit.
That said, I will also say that any worries about having merge commits in the history should largely be overcome. They actually provide useful information and are easy to ignore if you want.
That said, I will also say that any worries about having merge commits in the history should largely be overcome. They actually provide useful information and are easy to ignore if you want.
But again, the age-old response to this flow is that it creates an untrue version history. This is great if cleanliness is your exclusive priority, but if you find that you need to actually try to jump into the mind of the person who made the commit (especially if it's someone you've never met and with whose habits you aren't familiar), it's impossible to actually follow along and recreate what really happened.
The rebase strategy and merge strategy both have their places - neither is fit for "whenever we do a pull request" for all people everywhere.
The rebase strategy and merge strategy both have their places - neither is fit for "whenever we do a pull request" for all people everywhere.
I don't follow because the rebase is done by the person who wrote the code and is proposing the pull request. They rewrite their local history, but that doesn't seem to relevant. They still have to get their code to work with the latest master and remain in control of it. When the PR goes through, it can be a standard merge to maintain history.
True, but while that person probably tested each original commit as they wrote it (at least to the extent the application compiled and didn't completely blow up), they probably didn't do the same for each rebased commit every time they updated to master. If the resulting history turns out to be broken, it isn't the end of the world, but makes bisecting harder.
I think rebasing is a good idea anyway, but it's a tradeoff.
I think rebasing is a good idea anyway, but it's a tradeoff.
There's no perfect solution and all of this still requires a fair amount of vigilance to get it right. But I would still argue that if you are proposing a PR, you've done the work to verify your changes work, tests pass, etc. If you later need to rebase your PR, you still should run tests, smoke test, etc.
Small issues can slip through, but on teams I've worked on, submitting a PR that is truly broken is really bad form. Sometimes that does mean submitting PRs is tedious, but that's just how it goes sometimes. Thankfully, it's not that often in my experience.
Small issues can slip through, but on teams I've worked on, submitting a PR that is truly broken is really bad form. Sometimes that does mean submitting PRs is tedious, but that's just how it goes sometimes. Thankfully, it's not that often in my experience.
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.