merging approved pull requests

Mads Kiilerich mads at kiilerich.com
Thu Dec 4 11:39:50 EST 2014


On 12/04/2014 11:49 AM, Thomas De Schampheleire wrote:
> Hi Mads,
>
> On Wed, Dec 3, 2014 at 4:31 PM, Jan Heylen <heyleke at gmail.com> wrote:
>> Hi,
>>
>> we are trying to setup a POC Kallithea system to be able to do code review.
>>
>> what I did:
>> * setup a repo A
>> * fork rep A to A'
>> * clone A', make changes, push changes, amend changes, force push changes,
>> all to A'
>> * create a new pull request from A' to A
>> * approve the pull request, close the pull request
>>
> I have some additional questions regarding such review workflow:
> - is the described workflow a recommended one, in particular the need
> to force push changes? Or do you suggest something else?

Not necessarily. It depends on how you want to work and how you want to 
use your VCS tool and how you want your history to look. Kallithea do 
not hardcode as much as bitbucket & co do. Kallithea mainly fits in as a 
tool to make the D in DVCS work.

I guess the need for force here is that they want to use hg amend to 
polish changesets based on review feedback. (Which is made easy by 
manualy setting a [phases] publish = False on the server.) Pushing such 
a new version of a changeset to replace the old one will create a new 
head on the server. That is something that could look like a user error 
or cause problems for other users of the repo so you have to use the "I 
am strong and know what I'm doing" option.

Evolve try to keep track of that meta data and will automatically 
obsolete the old version on the server and remove the need for force.

Others use work flow that is closer to what seems to be a very common 
github pattern: Don't care about history at all, just add new changesets 
until the PR looks nice and then merge it with all the false starts and 
nuclear launch codes that has been removed.

> - when the developer modifies his changes based on review comments
> (using histedit or commit --amend for example) and issues a new pull
> request according to the described workflow, the old anonymous branch
> would remain on the A' repo. Currently there is no way to delete it,
> unless if an administrator goes into that repo and strips the changes
> manually. Is that correct?

As mentioned above, you can enable evolve globally and let it obsolete 
changes. Kallithea (and the alternatives) do not yet have good support 
for evolve ... but it seems to be coming. Pushing with force and only 
caring about the topmost/bookmed head might work for you, especially if 
you see these repos as "scratch pads" where people can show their patches.

> What about making it possible to delete associated commits when
> closing/deleting a pull request in Kallithea?

IIRC, the update pull request functionality currently only allows you to 
add additional changesets on top of the existing PR. That "makes sure" 
the PR stays "on topic" but do not support history modifications.

It would be nice to have strip as a last resort feature. I am not sure 
it would be a good idea to do it based on pull request disposal. But if 
it is something you want in the work flow you want to enforce, then it 
could be done in local patches.

> - currently the creation of pull requests is an administrative action
> on the web interface. Are there plans to create a command-line helper
> / mercurial extension that handles the pushing and creation of the
> pull request towards Kallithea?

There is an incomplete PR with additional API for pull requests, but 
IIRC that is only for viewing pull requests. Adding API and command line 
for modifying it sounds like a good idea and would not be so hard.

/Mads


More information about the kallithea-general mailing list