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

[Mads Kiilerich]

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:
>
> 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 ...)

> 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.

> 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?

> 4. To be able to approve/reject revisions from the changeset list
> page, withing having to open each changeset.

Blindly? That doesn't sound safe. Then perhaps better do it from the PR 
page where you at least look at the aggregated changesets.

> 5. To view who last changed the state of a changeset on the changeset list.

Somewhat related: It is odd that PRs to some extent have assigned 
reviewers and need "consensus" voting, while changesets only have a 
single random reviewer "deciding" everything. Something should change.

> Finally, I got another question:
>
> Is it possible to make file-wide comments? It seems it isn't. Is there
> a recommended workflow for this? Perhaps commenting on the first line
> of the file? This is a bit annoying when diffs are very big (and
> Kallithea hides some of the files on the diff initially) or when the
> files are binary.

It could make sense to for example use line number 0 or -1 for that. I 
think that would be quite easy to implement and quite self contained.

/Mads