On Fri, Sep 12, 2008 at 12:35:33AM -0700, Jason Dagit wrote:
> On Thu, Sep 11, 2008 at 11:12 PM, Dmitry Kurochkin
> <[EMAIL PROTECTED]> wrote:
> >>> Dmitry Kurochkin <[EMAIL PROTECTED]>**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.

I don't recall that there's an actual bug where amend-record doesn't
properly respect dependencies.  It is true, however, that in darcs-2
semantics amend-record need not respect dependencies (and really
*should* not), but that's a feature that needs to wait for
get_common_and_uncommon to be replaced with something that works
properly with darcs-2 semantics.

> 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.

I disagree.  I think amended patches are generally better.

> 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.

The disadvantage of having multiple patches is that it often obscures
the change.  In some cases, there may be useful history in the various
revisions, but for most changes it just makes it very hard to see what
was changed, as it forces the reader/reviewer (whether reviewing
patches for inclusion, or examining them in trying to track down a
bug) to understand both the mistakes of the coder as well as the
actual changes made.

Multiple patches are definitely better when a change is made up of
multiple sub-changes that are independently verifiable, correct and
smaller than the combined patch would be, as that does make the
changes easier to read.

> > 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.

I also filp flop on this, and don't want a hard policy--or massive
changes in either direction--but feel that whoever is working on a bit
of code should make it so it's readable for him in his editor.  I do
prefer that we avoid excessive indentation when possible.  e.g. it's
usually nicer to have

foo baz bar (Repository bas sab oof) opts =
  do a <- veryLongFunction withLots of arguments ++ more arguments

than

foo baz bar (Repository bas sab oof) opts = do a <- veryLongFunction withLots 
of arguments ++ more arguments

But it's also nicer to avoid unneeded line breaks, so I flip flop.  I
do like

foo baz bar (Repository bas sab oof) opts = do a <- short function
                                               other function a

which is very nice and compact, but then sooner or later someone
introduces a long function into that do block.

So in other words, I can spend a lot of words writing very little on
this subject....

David
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users

Reply via email to