comments on summary versus commits

[Jan Heylen]

inline

On Sat, Jan 24, 2015 at 7:24 AM, Nick Coghlan <ncoghlan at gmail.com> wrote:
> On 24 January 2015 at 04:56, Mads Kiilerich <mads at kiilerich.com> wrote:
>> On 01/23/2015 04:09 PM, Jan Heylen wrote:
>>>
>>> Hi,
>>
>>
>> I mostly agree with your thoughts. It would be nice to have improvements in
>> the areas and directions you mention.
>>
>> Some quick comments:
>>
>> I would like to improve Kallithea so it pushes users towards creating nice
>> commits - not just piling random hacking up until it looks fine but has a
>> messy history.
>>
>> I think that ideally, review should happen per changeset. The full "pull
>> request" diff can only be used to give a summary of what really happened.
>>
>> One step in that direction would be to include some annotate information in
>> the pull request diff, and instead of making comments on the pull request,
>> make a comment on the PR changeset that changed the corresponding line. It
>> should of course also flow in the opposite direction so a pull request
>> aggregates comments from all changesets.
>
> I think focusing the review on the individual commits rather than the
> whole feature branch proposed for merging is one of the key things
> that Gerrit does well relative to the GitHub/BitBucket style pull
> request model.

My reference tool is indeed Gerrit, coming from a git/gerrit setup
towards a Mercurial/Kallithea setup.

>
> To let you sensibly use git's history editing and rebasing features
> during the review process, Gerrit lets you add a "Change-Id" marker to
> the commit message, so it knows when your commit is actually a
> revision of an earlier one (that's necessary for git, for Mercurial
> changeset evolution should provide a similar capability natively).

The evolve extension looks indeed powerfull, having the change-id
built-in into Mercurial.

>
> This ends up being really valuable for complex feature branch merges,
> since you can do the main review once, and then review the "diff of
> the diff" to check that review comments have been addressed before
> merging it.

Something similar is already possible in Kallithea, but not with the
evolve extension (or any change to the commits in a pull request). If
you push new commits on top of existing commits that are in a Pull
Request, Kallithea proposes you to update the Pull Request, and in
that case you can get a diff of the new pull request against the old
pull request, as this is only when adding new commits and not
'amending' the original ones, it is not per commit naturally...

So the main effort will probably lie in the support for evolve in
Kallithea, and being able to update/diff pull requests based on the
evolve information.

>
> Regards,
> Nick.
>
> --
> Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia

best regards,

Jan