Kallithea issues archive

Issue #245: [feature] Show changes to Destination repository in a Pull request after Destination repository is changed

Reported by: Mario Juric
State: new
Created on: 2016-09-21 12:50
Updated on: 2018-05-18 20:20


This is what I understood so far on how Pull request process works in Kallithea. Once a pull request is created a common ancestor between Origin repository and Destination repository is conceived and the diff is calculated and shown based on this ancestor.

In case something is changed to the Origin repository of the pull request the Pull request page will show this and there will be an option to open an updated pull request (v2) and the previous pull request will be closed.

However if something is changed in the Destination repository no such information exists, which would be pretty interesting to know, since some time can pass before a Pull request is reviewed and there might be changes added to the Destination repo that are NOT show in the Pull request diff.

So is there a way to either change the ancestor to include the new changes made to the destination ( Which is what I would expect to work out of the box, since when I create a pull request I use a branch not a specific revision ), or add some kind of info like it's done when creating a pull request, where there is an information that tells if the Destination has some commits ahead of Origin.



Comment by Mads Kiilerich, on 2016-09-21 14:26

I assume you mean Destination in the title?

The way to update the ancestor is currently to merge the destination revision to the origin and make a new iteration (called "update" in 0.3.x). That is very explicit but gives good tracking of what changed.

Other strategies could also make sense in some cases but that is not implemented yet.

Comment by Mario Juric, on 2016-09-22 07:33

Comment by Mario Juric, on 2016-09-22 07:44

Hi @kiilerix

yes I meant 'Destination' in the title, thanks for noticing that :)

I see what you mean by merging Destination to Origin, and that's perfectly fine, but it would be nice that there is some indication in the Pull request details page that Destination is X commits ahead of Origin, so the reviewer of the pull request and the owner of the pull request know that they should merge the changes from Destination to Origin and 'update' the pull request.

This is nicely shown when the pull request is being created but once it's created there is no such information. In a situation where there are a lot of pull request and it might take some time before a pull request is reviewed and merged it can happen that Destination gets updated without reviewer and the owner knowing.

Do you think this is something that can be added to the Pull request page. As far as I can see ( from a user perspective ) this already exists when the pull request is being created, not sure how much effort would it be to show the same thing in the Pull request page?

Also, are there any plans on showing a possible merge conflict in the diff view, similar to what Bitbucket has?


Comment by Mads Kiilerich, on 2016-09-22 10:58

Yes, it could make sense to show how much the PR is "behind" the target. A bit like we already do for the origin - only different ;-)

Showing a "merge preview" is quite expensive and come with some caveats. Also, there is a bit of a philosophical difference there ;-) Technically, it kind of depends on Mercurial "in memory merge" from @seanfarley . I don't know what it would take on the Git side.

Comment by Andrej Shadura, on 2016-09-22 12:13

Actually, not at all, showing a merge preview isn't expensive as it can be precalculated once and used multiple times. I thought about implementing it for both Git and Mercurial, and I have some idea on how to do that, but I haven't found time to actually implement it.

Comment by Andrej Shadura, on 2016-09-22 12:17

Also, on the Git side it may be even easier, as the repository itself can be used to store the merge result without it appearing in the history.

Comment by Mads Kiilerich, on 2016-09-22 12:23

It has to be recomputed for all PRs in all forks every time the main repository changes. So yes, it is expensive, also with precomputing or caching. But that does of course not prove that it isn't worth it for you ;-)

Comment by Sean Farley, on 2016-09-22 17:37

A few notes (mostly drive by since I haven't finished my coffee):

  • git is fairly easy (you can do it out of tree and cache that)
  • Mercurial requires my in-memory merge work (I swear I will do it one day!!)
  • calculating the merge is cheap ... except when it's not (large repos, binary files, etc)
  • locking the repo due to the merge causes us many headaches:
    • every push has to figure out which PRs to recalculate
    • every time you merge a PR, you need to recalculate the merge commit (since the target is usually 'master') for every other PR (this makes having many open PRs slow as shit on Bitbucket)
    • potentially a new PR state of "calculating preview" (we didn't do this because no one wanted to touch the UI)

Comment by Andrej Shadura, on 2016-09-22 18:23

What if (don't shoot me!) we use Git to do Mercurial merge previews?

Comment by Mads Kiilerich, on 2016-09-22 18:31

I would mainly do it the other way around to use Mercurial for revsets - but with two-way mirroring so both sides would work equally well and users could use the VCS client of their choice. That would also cover that use case ;-)

Comment by Thomas De Schampheleire, on 2018-05-18 20:20