[PATCH] pullrequests: add support for custom pull request id prefix

[Thomas De Schampheleire]

On Sun, Apr 19, 2015 at 6:16 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 04/19/2015 03:54 AM, Thomas De Schampheleire wrote:
>>
>> On Sun, Apr 19, 2015 at 12:20 AM, Mads Kiilerich <mads at kiilerich.com>
>> wrote:
>>>
>>> On 04/18/2015 04:11 PM, Thomas De Schampheleire wrote:
>>>>
>>>> # HG changeset patch
>>>> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>>>> # Date 1429265659 -7200
>>>> #      Fri Apr 17 12:14:19 2015 +0200
>>>> # Node ID 6e760af6050e567239155a7a43b5ab02eddf877d
>>>> # Parent  32de32f6946110ecd0b8d2c79f0270025f3afaca
>>>> pullrequests: add support for custom pull request id prefix
>>>>
>>>> Currently, a pull request id is referenced as #5, and the '#' symbol is
>>>> fixed.
>>>> This commit adds a configuration parameter 'pull_request_prefix',
>>>> defaulting
>>>> to '#', that can be used to customize the prefix.
>>>>
>>>> For example, one could use 'PR-' to create references of the form
>>>> 'PR-5'.
>>>
>>>
>>> Yes, the # notation is weird.
>>>
>>> But we can't make everything configurable. I think it would be better to
>>> just use 'PR-' everywhere.
>>
>> My use case is this:
>> we are planning to run different Kallithea instances, one in each of
>> our (geographically separated) sites. The reviews (pull requests) are
>> supposed to be traceable throughout history, so references to such
>> reviews are to have unique IDs across these different Kallithea
>> instances. Since they do not share the same database, there will be
>> several pull requests #4. Same for PR-4.
>>
>> Therefore, we plan to use a pull request reference that contain a
>> reference to the site, for example: PR-<site>-4. This patch is a
>> solution that supports this idea in an upstreamable way, without too
>> much complexity.
>>
>> Would you reconsider this patch after this background?
>
>
> Hmm. Dunno ... I'm not fond of adding 'random' configuration knobs all over
> ... but ...
>
> It would be nice to have an extension mechanism. Perhaps not as
> sophisticated as Mercurial's. Perhaps just a way to import a set of py files
> that then could import other modules and monkeypatch them. Such "extensions"
> could be very simple.

It sounds like a good idea to me: it would solve the problem that some
companies require tweaks here and there, but directly changing
Kallithea code that cannot be upstreamed means a more involved upgrade
procedure. With such an extension mechanism these small tweaks live
alongside Kallithea core code.

I'm not familiar with monkey patching in Python. Would it be
sufficient to import the patching code in one place, which place
should this be? Maybe kallithea/__init__.py?

This code could import all .py files found in a given subdirectory,
for example monkeypatch/*.py. Then companies can structure their patch
code as they wish, without needing it to be all in the same file.

>
> I would probably take a patch that moved the hardcoded # naming to a method
> on the PullRequest db model, similar to how we have .url . It would be very
> much like the existing .pull_request_id field so perhaps name it .nice_id?

Shouldn't it be symmetrical: pull_request_nice_id or something like that?

>
> It is a bit weird that Kallithea pull request numbers are global. Especially
> in a site that is hosting repos for multiple independent users, it would
> make sense to have per repo numbering. Would that solve your case? Will your
> repos in the different instances be named differently?

No, the different instances would operate on the same repositories
with the same names (note that we're not using Kallithea for repo
hosting, it is a mirror).

Also, global IDs have the benefit of being completely standalone. If
we have per-repo IDs, I cannot tell you 'look at pull request 5',
because I would have to tell you which repo to look in.

>
> Another solution could be to tell the database to start the sequences with
> different starting points (like 1000000) and perhaps add database
> constraints to enforce it.

You mean a different starting point for each instance?
It's a technical solution to have unique IDs per site, but not very
user-friendly: if a commit references a pull request 20004561, it's
not at all obvious to which URL to go to. A prefix based on a site
name is more clear in this respect.

Thanks,
Thomas