[PATCH] pullrequests: saving raw_id instead of branch in org_ref
Thomas De Schampheleire
patrickdepinguin at gmail.com
Fri Mar 20 08:18:14 EDT 2015
On Fri, Mar 20, 2015 at 1:24 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 03/19/2015 07:59 PM, Thomas De Schampheleire wrote:
>>
>> On March 19, 2015 3:38:39 PM CET, Mads Kiilerich <mads at kiilerich.com>
>> wrote:
>>>
>>> On 03/19/2015 03:00 PM, Jan Heylen wrote:
>>>>
>>>> # HG changeset patch
>>>> # User Jan Heylen <jan.heylen at alcatel-lucent.com>
>>>> # Date 1426767186 -3600
>>>> # Thu Mar 19 13:13:06 2015 +0100
>>>> # Node ID beae6b3ec2fc556a274630de6d7b02b1cf54052a
>>>> # Parent b08aab61c41d60562f0033f927cf32c8e024e24b
>>>> pullrequests: saving raw_id instead of branch in org_ref
>>>>
>>>> This fixes an issue when the pull request is created while the top
>>>
>>> commit
>>>>
>>>> included is not the tip. In that case, the faulty code would but
>>>> branch:default:default as org_ref, that causes the pull request view
>>>
>>> to show
>>>>
>>>> all commits on that branch instead of the wanted commit(s) only. The
>>>
>>> fix will
>>>>
>>>> result in a branch:default:1234567890abcdef1234567890abcdef (e.g)
>>>
>>> Hmm ... it should be using org_rev when computing cs_ranges. The ref
>>> should only be used when finding the list of changesets that can be
>>> used
>>> for "update" ... and I think it makes sense to show new changes on the
>>> branch (or bookmark) even when creating the PR on a specific revision.
>>>
>>> I will have to investigate more ... unless you can say more about it.
>>
>> To be clear, the list of commits in the pull request is fine, but the
>> overall diff shown
>> on the pull request page also contains newer changes on the branch that
>> are not
>> part of the pull request.
>
>
> Right. revisions / cs_ranges is populated correctly, but the diff will use
> cs_rev which comes from the last part of the ref. (It should perhaps use
> cs_ranges[-1] for diffs instead of using the ref. See also the TODO in
> pullrequest_show.html.)
Regardless of this TODO, I think the patch is fine in removing a
different behavior depending on the fact that the PR head happens to
be the tip at the time of PR creation or not.
More information about the kallithea-general
mailing list