Integrating Kallithea with TortoiseHg (plus some other comments/suggestions from my team)

[Thomas De Schampheleire]

On Mon, Feb 15, 2016 at 12:04 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 02/14/2016 07:38 PM, Angel Ezquerra wrote:
>>
>> Hi,
>>
>> on Friday I showed Kallithea to the people on my team and most really
>> liked. However some said that they'd prefer to be able to do some (if
>> not all) of the code reviews directly in their TortoiseHg client,
>> which is what we use as our mercurial client.
>
>
> Yes, that could be convenient. We have considered it too. A first challenge
> was that most people actually rather would have integration with their
> editor or merge tool ... or doesn't use TortoiseHg.
>
> I think the right answer to that (and other problems) is that review
> comments really should be "first class objects" in the VCS. They should work
> the same across all UIs ... and they should do The Right Thing when
> changesets are amended or rebased.
>
>> In particular, these were some of the things they requested:
>>
>> 1. To be able to open the Kallithea review page for a given revision
>> from TortoiseHg
>>
>> This should be pretty straightforward to do. I just need to add a
>> Custom Tool that opens the corresponding URL in Kallithea.
>>
>> Ideally it should be possible to jump straight to the bottom of the
>> code review page, where the Approve form is found. Looking at the
>> current code I could just use the main_form id as the anchor for the
>> html, that is:

Odd request, see below.

>>
>> http://kallithea-server:port/REPOSITORY_PATH/CHANGESET#main_form
>>
>> Can we expect the main form id to remain unchanged in the foreseeable
>> future?
>
>
> I guess it is very likely we want to totally rework that page soon. But when
> you control both client and server in your installation, you can always
> tweak it to match eather or be backwards compatible.
>
>> In fact, it would be nice to be able to quickly access the approve
>> form from the top of the page. Perhaps the "[Not approved]" text that
>> appears at the top of the review page could be a link to the approve
>> form?
>
>
> That text is generally referring to that _other_ users didn't approve yet.
> Having the link you describe could be confusing.
>
> But also, it seems wrong to encourage people to approve before looking at
> the diff?
>
> (But also, Jan and Thomas have tweaks that kind of goes in the same
> direction ...)

It's a different goal, though.

I find the request to skip directly to the approve button very
strange, to be honest. Code review is not just about
approving/rejecting, but also on giving code comments (which are most
useful when placed inline in the code, really).

Why would you want to skip to that button and give the impression that
you cannot/should not comment on the code?

>
>> 2. To be able to jump straight to the review section corresponding to
>> a particular file from TortoiseHg
>>
>> This does not seem possible at the moment. It seems that the anchors
>> corresponding to changeset files on the changeset review page are not
>> easy to generate. Ideally they should have a format of the sort
>> CHANGESETID-URL_ENCODED_FILEPATH but they do not seem to follow that
>> sort of convention :-(
>
>
> Yes, I don't know why it is exactly the way it is. But the filepaths can be
> very long and are currently repeated a lot of times. It could be changed ...
> but should be done carefully as it would break links in existing comments
> and mails and chats. You can perhaps from the beginning use a different
> format in your setup. Or perhaps tweak it to have two set of anchors?
>
>> 3. To be able to mark a revision as "Approved" (or Rejected, etc) from
>> TortoiseHg
>>
>> Step #1 above already kind of solves this request, but if we wanted to
>> go for a more integrated solution it would be nice if we could do one
>> of two things:
>>
>> - Open a page that only had the approve form for the selected
>> changeset, and nothing else. We could then open it from TortoiseHg.
>> - Use the JSON API to directly approve or reject a changeset. I
>> checked the current JSON API but I did not see anything in there that
>> would allow to do this. Ideally there should be an API call that let
>> you set the revision state. We could then call it from TortoiseHg
>> (using a custom tool or with a more integrated solution).
>
>
> Yes, that could make sense.

Again, I find it a very strange request. You'd essentially bypass all
the good things that code review can bring you. Do you expect that all
users will add all their comments about a change in the one box near
the approve button? I think you'll get better quality reviews if
people actually comment on the lines of the code.


>
>> BTW, is is possible to download the stand-alone kallithea-api command
>> line tool from somewhere?
>
>
> It is quite integrated with paster and the rest of Kallithea.
>
> But look at the curl example - you barely need a tool.
>
> Or perhaps it would be better to put a python 2-liner in the documentation?

You can use the python 'requests' module,
http://docs.python-requests.org/en/master/

quite easy to implement and quite self contained.


/Thomas