-
Suggestion
-
Resolution: Fixed
-
188
-
We collect Bitbucket feedback from various sources, and we evaluate what we've collected when planning our product roadmap. To understand how this piece of feedback will be reviewed, see our Implementation of New Features Policy.
In stash 1.3, if you push a branch, and then (usually in response to the discussion) amend or rebase the branch, then push -f to updated, the pull request discussion shows added + removed commits.
However, there is no obvious way to compare the original state of the branch with then new branch to see what changed.
[BSERV-2896] Diff between original commit and amended/rebased commit
Hi all,
As mentioned above, this has been fixed by a change in Bitbucket Server and Data Center version 7.11.1.
There is now a "view changes" link on push activities in a pull request, which takes you to the diff view and shows the effective changes that were applied to the pull request during that push activity. This includes force pushes and other scope changes, such as when changing the target branch of the pull request.
Thanks,
Wolfgang Kritzinger
Bitbucket Senior Developer
This now exists in Bitbucket Data Center and Server version 7.17.2.
There is a new "view changes" option.
+1.
It is really frustrating to see a basic feature like this not being available after the problem was reported 8+ years ago.
Came here just to see if anyone at Bitbucket is working on this. Sad to find an 8 year old stale ticket...
Gitlab and Github have had this for years! Please 🙏
I fully agree with @Antoine Morisseau.
I just sent a feedback on Bitbucket for this very feature.
I cannot believe it's not in the new pull request experience feature list : https://support.atlassian.com/bitbucket-cloud/docs/try-the-new-pull-request-experience-in-bitbucket/
Gitlab does it for years and it's very useful to keep a clean commit history.
Maybe create a BCLOUD ticket for this feature ? could @jarredc see this and take care of it ?
I can add github to the list of git providers that support this feature (So now we know that gerrit, gitlab and github all support it.)
I'd like this feature to be supported, does anyone know the latest news?
Already, the Gitlab is supporting. "Compare with previous version".
Git 2.19 ([introduced]https://lwn.net/Articles/764552/) a new feature called [range-diff|https://git-scm.com/docs/git-range-diff], which would be at least a workaround for the lack of a feature that allows for comparing different versions of pull requests / patch sets. This would require though that Bitbucket kept earlier commits (i.e. earlier versions of the pull request / patch set) around and provided clients with access to those earlier commits after pull request is updated. And clients would of course need to have Git 2.19+ available. Many (Linux) distros may still ship an older version though.
I believe range-diff also removes the need to have and maintain Gerrit-style Change-Id's in commit messages in order to implement comparison of commit ranges / patch sets.
We are running into this currently where SOX auditors want to manually compare differences between commits of different releases.
A big +1 on my behalf. Personally I’m constantly longing for this feature.
Although earlier comments already cover the feature very well, I’ll re-iterate the need from my point of view.
I’ve also used Gerrit[1] in the past and this was the first difference I noticed once I started using BitBucket. From my point of view the primary benefit of being able to see the differences of a pull request / patch set compared to its predecessors is that it speeds up the review by allowing for identification of the changes the author did since I’ve previously reviewed the pull request. The actual review is typically done against the master though.
BitBucket does identify the changed / updated files by using a bold font but that is first of all hard to know for a new user as it’s (IMHO) not the most intuitive feature, and more importantly you only know what files were changed but don’t get any more fine-grained information. Changes to a pull request / code may be very subtle but still crucial and one essentially always needs to re-review the whole file BitBucket has identified (bolded) especially if the previous version was reviewed already a while ago. There is also the psychological aspect that having to re-review pull requests many times without knowing what exactly changed may reduce motivation and increase the risk of rubber-stamping changes after a certain number of review rounds.
The workaround I’ve personally used sometimes is to fetch each version of a pull request to a local clone after reviewing the pull request if it is anticipated that there will be changes to the pull request. This allows for comparing a new version to the previously fetched version(s) locally but this is far from ideal and laborious enough to make me usually avoid it.
It is true that rebasing a pull request / patch set on top of a different base makes the comparison of pull request / patch set versions typically difficult or even useless due to the noise of unrelated changes in the diff. In a merge workflow (vs. rebase workflow) this problem can typically be avoided by not rebasing to a new base after opening a pull request. This might even be preferable in some cases as the merge then records the version the developer / author used while she was developing and testing the change and also records any differences to the head of the target branch after the developer did her testing. This is more probable in large projects / repositories with tens of developers where the head moves quite fast but developers seldom change same files. Typically one can’t / won’t redo all testing after a final rebase just before the merge – especially if not all testing is automated. And in low-traffic repositories one doesn’t necessary need to use a merge workflow to benefit from the ability to diff pull request / patch set versions.
The ability to compare different versions of a pull request / patch set has proven to speed up my reviews many times. It may not always be useful but at least in my environment and workflow that has been the exception rather than the rule. And in that case one just needs to do without the ability to diff, which is the current BitBucket behavior. I.e. this should be an additional “opt-in” feature.
[1] Gerrit allows the user to freely compare any version of the patch set to any other version. See https://gerrit-review.googlesource.com/Documentation/user-review-ui.html#side-by-side, the row ‘Patch set Base 1 2 3 ...’ at the top of the screen. Live examples can be seen e.g. on the Eclipse Gerrit instance at https://git.eclipse.org/r/#/c/121793/6..12/core/org.eclipse.rcptt.core.builder/src/org/eclipse/rcptt/core/internal/builder/MigrateProjectsJob.java (not sure if the link to an individual review is valid indefinitely though).
Bitbucket server v5.6.2.
Just adding a +1 to this feature request. Could have used it this week - would like to use it in the future. Thanks.
jeremyhu97404818 - read comments above. It doesn't yet solve force pushes or rebases, but that's something we're keen to do in the future. Note that we could show arbitrary diffs between two commits, but a rebase onto a new base means that we'll be likely to show a lot of unrelated changes and confusing noise in that case. We'd need to detect and clearly message the problem there to avoid creating a footgun for people viewing a diff like that. Definitely something for us to look into. Thanks for the feedback!
How does the iterative review in 4.11 deal with commits that have been removed via force-push since an earlier commit? This is one of the very few places that phabricator is much much much better at supporting reviews. With phabricator, there is UI for selecting arbitrary tip commits (some possibly orphaned) and comparing them. That's what Bitbucket really needs.
kyletightest, the difference between commit-level review (in 4.8) and iterative review (in 4.11) is that multiple commits pushed since you last reviewed can be viewed (and reviewed) as a single diff. If there was only 1 commit pushed, then it's effectively the same.
kaveh1703963922, thanks for the frank feedback. Indeed it is difficult to do this properly due to a number of complexities that crop up, both technical and in terms of UX. We're not shying away from working it out at some point, especially now we've got the basic functionality in place. That said, like every software team we have a limited number of people and prioritisation comes into play. In the past year we've resolved 20 suggestions that were more highly voted than this one, amongst many other improvements. We'll continue to add what we feel are valuable improvements for as many people as we can, and your continued feedback and votes help us do that. Thanks again
@rbarnes Even better than tracking the suggestion would be implementing it. I can't imagine it would be difficult for Atlassian to do this, and I (and many others here) don't understand why it's taking so long.
How exactly has the pull request functionality changed? I have 4.8 and I was always able to pick a particular commit to see the changes made. So the author would make changes based on the comments and then commit, and id pick that commit in the PR to see the 'comment-based changes'.
Is it just that the 'last changes since you've reviewed' is now automatically shown when u open the pull request?
kaveh1703963922, you are correct, we closed this one in error and have now reopened it. The iterative review functionality that just shipped would need to be extended (somehow) to cover force pushes. We'll continue to track that suggestion here.
If this doesn't show you the difference between the branch as it was and the rebased branch as it is now, then how does it solve this problem? (Hint: it does not)
We are very excited to announce that iterative review has been shipped in Bitbucket Server 4.11.0! As a reviewer, you are now able to return to a pull request and see all of the changes that the author made since you last reviewed (in the case where the branch was amended). To find out more, take a look at the release notes for this release.
This however will only be shown to any reviewers of a pull request of that branch, and won't be shown in the case where the last reviewed commit was rebased.
Stefan Petrucev
Atlassian Bitbucket
you bring up garbage collection as a reason not to do this - but doesn't stash disable garbage collection and never actually run it? You can only run it manually from the server, and if you do that without pausing stash, or especially if you put it in a crontab, you enable race conditions. Because of the way stash forks repos, I'm pretty sure, using alternatives, it makes safely using garbage collection nearly impossible. Is that even on you guys' roadmap?
EDIT: it looks like this changed or I was mistaken because GC is not disabled on the stash repos - although the reflog is set to never expire, which I am pretty sure means the commits that were once reachable will remain always reachable.
That also solves my comment about GC being unsafe, because if refs are never removed, the race I talked about can't happen. The change was probably in response to that?
One solution to this would be for Stash to take snapshots of the PR branch each time said branch is updated (snapshots in the form of patch files). You could then rely on your patch files to view diffs, offering "update revisions" to the PR, similar to how ReviewBoard does it.
Viewing the diffs of each individual commit is still possible, but you'd be able to diff the tip of the branch across your snapshots. This would allow you to view the diff of the branch even if ancestry is rewritten (aka force push).
Hi,
I'm also looking for the same workflow.
Commit Id 1: c3089cae138
git add <file>
git commit –m “<message>”
git push origin <branch_name>
Commit Id 2: f7a455b694b
git add <file>
git commit –amend
git push origin <branch_name> (Not Working)
Using :: git push –force origin <branch_name>
In Stash GUI, the commits are viewable as Individual commit, not showing as PATCH-SETS(just as in GERRIT) with history and diff in single web page.
1. whether it's possible to get the GUI just like GERRIT for seeing the PATCH sets of particular change in the single Page.
This functionality is critical for our project as we commit the change & get that reviewed , go back & forth with review changes addressed and listed as patch sets.
Please let all know any other plugin/hooks available for the same or whether we can get this functionality implemented on our own by writing the new hook on our own using https://developer.atlassian.com/stash/docs/latest/how-tos/repository-hooks.html?continue=https%3A%2F%2Fdeveloper.atlassian.com%2Fstash%2Fdocs%2Flatest%2Fhow-tos%2Frepository-hooks.html&application=dac
I've got multiple development teams at my company, and the lack of this feature is preventing us from migrating to Stash. Any hope this of STASH-2550 are going to come out anytime soon?
The lack of this feature is made worse by the lack of arbitrary commit diffs ( https://jira.atlassian.com/browse/STASH-2550 ) and the fact that the UI tells me which commits were removed and which commits were added. The hashes are right there, we just want a basic diff... it's so close and enables a bunch of use cases as listed here.
I'm sorry for the lack of updates on this. Keeping hundreds of open public suggestions up-to-date with status is rather time-consuming, but we're going to put more effort into it in future.
Of course it is provocative and false to suggest that Stash is only for casual code review. If that were the case we'd not have thousands of customers large and small using it to ship quality software.
We are working on a range of improvements for pull requests in Stash, and these will absolutely take into account suggestions for reviewing what's changed throughout the life of a pull request. We don't have a definite timeline for such improvements at this stage.
Stash is only meant for a casual glance at code and not for serious code review
Is that true Atlassian?
@Hannes de Jager: This issue is two and a half years old and gets no attention. Clearly, Stash is only meant for a casual glance at code and not for serious code review. If you care about reviews, my recommendation is to go back to Gerrit.
We have two teams that moved to stash from Gerrit and I must admit for a lot of us, not having this feature is really annoying and often raises the question: "Should we go back to Gerrit". But then, Stash is so awesome in most other regards. Really good job guys, just seriously miss this... It seems like Crucible have this. In fact that is the only reason we started evaluating Crucible, is for this. But Crucible it totally overkill for our use case. Again miss this ... please guys
Our use case is similar (as we discussed in person). Typical workflow for a code review is:
- Generate the pull request
- Get feedback
- Fix and push to branch (generally by amending or rebasing the branch, this keeps overall commit history cleaner once merged into the main branches)
- Get feedback
- Fix more and push to branch
- ...rinse and repeat until approval
- Approve and merge.
I agree that this could be achieved through adding refs.
As a work-around, we push fixup commits in Stash, but that's not really good enough. It's not always obvious to the reviewer which fixup is meant for which commit, it's not possible to see the combined effect of a commit and its fixups (only the combined effect of the entire branch), it's easy to forget to squash the fixups before merging (we have many "fixup! foo" commits on our develop branch we won't get rid of), the reviewers never see the actually intended commit list, and it's not always even possible to make a fixup commit.
As an example of when fixups don't work, consider a change where the committer wants to change a timeout on some line. The current timeout is written as a constant, directly when it's used. The committer wants first refactor by pulling the constant out and name it, and then change its value as a second commit. These two committs are pushed for review, and a reviewer points out that the name doesn't actually match with what the timeout is used for. The closest thing you can do with fixups is to push a fixup that renames the constant but has the new value, and then when squashing moving the fixup to the first commit and solving the merge conflict twice - for no gain.
All these issues are solved if the committer updates the commits and force-pushes, but then, in Stash, the reviewers can't see what changed - unless this feature request is implemented.
So the normal workflow for me when I was in a team that used Gerrit was that first someone pushes an initial attempt at a commit. I and others give comments, and the committer responds to them, and pushes up a new version of the commit, with some changes due to the comments. Now I want to see that those changes match what I was asking for, and that any other changes to the patch are acceptable. I do not want to read the whole thing again, because all other changes have already been accepted. If the patch is still not good enough, we do another round of comments, updates, and re-reviews. Typically, we did maybe 3-4 rounds on a patch, but there were outliers where the initial attempt went in as-is, and on the other side of the scale, some with more than 10 updates. Since the majority of the times we looked at a patch, we looked at an update of an older attempt, so having the tool show us just what was new really helped (though, of course, rebasing the commit in the middle of updating generated a lot of noise, and was hence discouraged - you push, fix, re-fix etc. until accepted, /then/ you rebase and merge).
The diff-against-previous was the Gerrit view I used the most, in terms of page loads. In terms of time spent, sure, that was on the diff against master, but that's because Gerrit allowed for fast re-reviews. In Stash, I waste a lot of time jumping between the comment list, the latest diff, and the commit list, trying to figure out what the committer actually changed this time, and if all the comments were addressed. A good implementation of this feature request would solve that.
We would love to get more feedback on specific use-cases for this feature. Under what circumstances would you use the resulting diff?
Should be possible to set up refs to all related commits when updating the review. That way, they cannot be garbage collected.
Gerrit manages this quite nicely by adding a ref for each submitted patch.
After amending or rebasing the branch, older commits are up for garbage collection and thus may be removed from the repository. As such it would not be possible to show the diff between the original state and the last revision.
e43651f12c0f: On the pull request "Overview" tab in the "Activities" section, on every activity that updates the scope of the pull request, such as a push or a rebase, it says "<Name> UPDATED the pull request by adding/removing X commits (view changes)". The "view changes" link is there.