[darcs-users] darcs patch: Formating and minor refactoring in URL.u... (and 8 more)

[Jason Dagit]

On Thu, Sep 11, 2008 at 11:12 PM, Dmitry Kurochkin
<dmitry.kurochkin at gmail.com> wrote:
>>> Dmitry Kurochkin <dmitry.kurochkin at gmail.com>**20080910061227] hunk ./src/URL.hs 118
>>> +            debugMessage $ "URL.urlThread ("++(url r)++"\n"++
>>> +                             "            -> "++(file r)++")"
>>
>> Since we were doing this anyway, we could have removed the excess
>> parentheses here, which helps readibility a tiny bit: ++ url r ++ "\n"
>> ++ file r ++ ")"
>
> Done, in a separate patch though. Could not amend-record because of
> patch dependencies. Is there a walk around for this?

One thing I've noticed is that amend-record isn't as good at letting
you select patches as other commands.  For example, if you try to pull
your changes to a different copy of the repository you can be more
selective when picking your patch and if there are dependencies you
find out after picking instead of during.  The next step in this flow
is to unrecord, fix things, and finally record again.  Or, if you
don't need cherry picking, then just plain unrecord seems to work a
bit better than amend-record.

Probably a rewrite of the amend-record patch selection code is in
order.  I think David actually hinted at this in an email conversation
I had with him recently when discussing how to refactor amend-record
for type witnesses.  It would appear that amend-record uses a special
function for patch selection that is not shared with other commands.

Then again, if you're stuck on actual patch dependencies then the only
solution is to also unrecord the depended upon patches and regenerate
the depended patches.  Perhaps amend-record could be updated to allow
this, but it would make amend-record even more unsafe than it already
is, especially if it didn't notify you that the depended patches had
been modified.

My other comment is that, I think with the exception of typos and
unforeseen flaws that it's actually healthier for us to have
refinement patches than amended patches.  For example, the extra
parens are not a mistake but not having them is nicer.  So, I think my
preference would actually be to have both patches get accepted instead
of having you amend the first patch.

If there are technical reasons to prefer the amended patch then we can
hopefully address those with an improvement to darcs.  If there are no
technical reasons then I don't really see any disadvantages to having
both patches.  If anything, having both patches documents that we like
to keep things looking clean in the source.

> I have obliterated this patch since now (after the above change) these
> lines are not that long. But in general I do not like when lines are
> more than 80 columns.

I flip flop on this style point myself.  It is nice when lines are <
80 columns, but sometimes it is more hassle to enforce that than it is
to just deal with a line or two that is, say, 90 columns.  Having said
that, it's well known that people, including yourself, will spend
vastly more reading your code than you spend writing it so you should
write for the benefit of the readers.

Thanks for your hardwork Dmitry!

Jason