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

[Thomas De Schampheleire]

On Mon, Apr 20, 2015 at 11:55 AM, Thomas De Schampheleire
<patrickdepinguin at gmail.com> wrote:
> 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.

Note: to make this really work, we should add a URL that accepts a
pull request ID alone. Currently the pull request URL requires the
repo already, so today my argument is not fully correct.

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