pull requests: variable names

Nick Coghlan ncoghlan at gmail.com
Thu Feb 19 05:07:12 EST 2015


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.

Regards,
Nick.

-- 
Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia


More information about the kallithea-general mailing list