Felipe Contreras wrote:
> On Wed, May 1, 2013 at 1:36 PM, Ramkumar Ramachandra <artagnon@xxxxxxxxx> wrote:
>> This is not a reorganization patch. I said "simplify": not refactor.
>
> I'd say you should start with a reorganization, and then a simplification.
You don't think I already tried that? There is no way to sensibly
reorganize the old logic sensibly, in a way that doesn't break
anything.
>> I've changed the entire logic and written expensive comments; and I'm
>> not allowed to remove one comment which I didn't find helpful?
>
> But it is helpful.
Okay, we'll add it back if you feel strongly about it. I thought it
would be a sore thumb when neither @{N} nor @{upstream} are explained.
>> I'm claiming that there is no functional change at the program level,
>> with the commenting*. If you want to disprove my claim, you have to
>> write a test that breaks after this patch.
>
> The burden of proof resides in the one that makes the claim, I don't
> need to prove that there are functional changes, merely that you
> haven't met your burden of proof.
Oh, but I have. All the tests (along with the new ones I added in [1/5]) pass.
Scientific theories stand until you can find one observation that disproves it.
> That being said, perhaps there are no _behavioral_ changes, because
> you are relegating some of the current functionality to dwim_log, but
> the code most certainly is doing something completely different.
> Before, get_sha1_basic recursively called itself when @{-N} resolved
> to a branch name, now, you rely on dwim_log to do this for you without
> recursion, which is nice, but functionally different.
Your point being? That I shouldn't say "no functional changes"
because the logic is changing non-trivially at this level?
>>> It's not true, there might not be any @{u} in there.
>>
>> This entire structure is a success-filter. At the end of this, if
>> !refs_found, then there has been a failure.
>
> That's irrelevant, this 'else' case is for !reflog_len, there might or
> might not be @{u} in there.
There's no need to associate one comment with one line of code.
People can see clearly see the failure case following it.
>> The Git project is already suffering from a severe shortage of
>> comments [1], and you're complaining about too many comments?
>
> Irrelevant conclusion fallacy; let's suppose that the Git project is
> indeed suffering from a shortage of comments, that doesn't imply that
> *these* comments in their present form are any good.
Is there anything _wrong_ with the comments, apart from the fact that
they seem to be too big? I'm saying things that I cannot say in the
commit message.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html