[darcs-users] darcs patch: Resolve issue1093: warn about ugly patch... (and 1 more)

Eric Kow kowey at darcs.net
Mon Feb 2 10:32:22 UTC 2009


On Mon, Feb 02, 2009 at 20:31:39 +1100, Trent W. Buck wrote:
> > > +                     -- Note that we use isLower here instead of not
> > > +                     -- . isUpper because some languages are caseless.
> > > +                     | isLower $ head n =
> > 
> > Your comment is interesting.  So I presume this means that in caseless
> > languages, isUpper is always True and isLower is always False?
> 
> Well, that's where more testing than I did comes into play :-)

:-)

But was that your expectation?  Just trying to make sure I understand
the comment.  If so, I might rephrase so that it's very explicit what
happens in a caseless language, presumably the right thing being that
no warning is printed.

> The null would hopefully be caught by the earlier length n < 10.

Right.  That said, the pattern match is still a better idea in terms of
making the code more evolution friendly.  Suppose we later decide to do
away with the length check?  Then things go boom.  So I would advocate
maybe something like

 n@(n0:_) | isLower n0 = ...
          | last n = ...
 n | other conditions

> Well, that's why it's a warning instead of an error.  I wanted to give
> helpful hints to newbies (and tired Darcs developers!) so that they
> don't end up with a prestigious project that has a bunch of stupid /
> ugly change descriptions in the early (and thus unamendable) history.
> 
> Under what circumstances is it desirable for a patch name to *not* be
> a sentence?

Not all projects are prestigious and sometimes people just want to throw
together a quick little repository.  I've also flip-flopped between
putting punctuation and not (seeing it as superfluous), although now
that I hear somebody else saying that it's good to have, I'll be a good
boy from now on.  (That said, it's not clear if we always want a
sentence, maybe sometimes a noun phrase is more direct?)

I think this patch is a bit too helpful.  It's really just a gut
feeling, and it could be wrong.  My personal approval vote is:

 YES - warn on patches starting with '-'
 YES - warn on "strange" patch names http://bugs.darcs.net/issue1000 
 MAYBE - warn on overly long patch names (as this could cause some
         concrete practical problems for other folks, wrt wrapping)
 NO - warn on patch names < 10
 NO - warn on punctuation

I don't want this to turn into bikeshed either.  Maybe we could hear
from three more users?

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
Url : http://lists.osuosl.org/pipermail/darcs-users/attachments/20090202/b4557515/attachment-0001.pgp 


More information about the darcs-users mailing list