comments on summary versus commits
Sean Farley
sean.michael.farley at gmail.com
Sat Jan 24 03:00:48 EST 2015
Nick Coghlan writes:
> 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.
>
> 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).
>
> 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.
This is one of major visions I have for Kallithea. I just wish I had
more cycles to work on it.
More information about the kallithea-general
mailing list