pull requests: variable names

[Thomas De Schampheleire]

On Thu, Feb 19, 2015 at 11:07 AM, Nick Coghlan <ncoghlan at gmail.com> wrote:
> On 17 February 2015 at 13:13, Mads Kiilerich <mads at kiilerich.com> wrote:
>> On 02/14/2015 09:36 PM, Thomas De Schampheleire wrote:
>>>
>>> Hi,
>>>
>>> In the pull request code (both controllers and templates) I find the
>>> variable names non-intuitive and it is therefore harder to make
>>> changes.
>>
>>
>> I have tried to make names that was better than before ... but it seems like
>> there is some way to go ;-)
>>
>>> With this mail I would like to get an agreement on the terminology we
>>> can change to.
>>>
>>> - org vs other --> source/src and dest
>>
>>
>> Yes, that is a bit messy and could use some cleanup.
>>
>> I think the main challenge is that a pull request and compare in some ways
>> are looking at the same thing from different point of views and reuse code
>> and templates. The names source and dest are not so obvious in the context
>> of compare. But give it a try and see how it works out. (Consider doing the
>> two renaming in two different changesets so it is easier to review.)
>
> Ah, that does indeed make a difference - source/dest isn't right for pure diffs.
>
>>> - c.cs_repo: what does 'cs' stand for here? This could become src_repo
>>
>>
>> 'cs' is for changesets, referring to the side where the changes happened.
>>
>> I think the advantage of the current naming is that it refers to what it
>> _is_ not to what we might happen to use it for or how we interpret it.
>
> Would "changed_repo" be OK?
>
>>> - c.a_repo: what does 'a' stand for here?? This could become dest_repo.
>>
>> The 'a' was meant to refer to the ancestor used for comparing.
>
> And expanding this out to "ancestor_repo"?
>
> Now you've explained them, I think changed & ancestor make sense as
> terminology, but the "cs" and "a" abbreviations are arguably a bit
> *too* abbreviated for clarity.
>

Agreed.

Note that 'ancestor_repo' is not necessarily true: when comparing two
repos they could both be peers, not ancestor-child, right?

Also, in case of compare, change may neither be accurate.

I think it's hard to find one combination of terms that matches both
compare _and_ pullrequests.
Which code is actually shared between both?
If the terminology there is neutral, then in both compare and
pullrequests the appropriate derived terminology could be mapped on
it, for example left/right for compare, and src/dest for pullrequest.

Best regards,
Thomas