[darcs-devel] darcs patch: configure.ac: restore bytestring to default (and 25 more)

Jason Dagit dagit at codersbase.com
Thu Apr 17 16:22:36 UTC 2008


On Thu, Apr 17, 2008 at 8:28 AM, David Roundy <droundy at darcs.net> wrote:

> I'm rejecting this patch.  I'm not going to include cabalization changes
> that don't work (e.g. don't support ghc < 6.8), as I believe this will
> confuse users and/or reduce the amount of testing darcs' real build
> process
> receives.  I may not apply even a proper cabalization, but one that will
> not build darcs on my system is not even going to be under consideration.


@David:
I take it this means you don't like the idea of having someone else carry
the burden of supporting cabal?  I'm willing to do this and I was even in
the process of reviewing these patches.  Also, one of the reasons to use a
buildbot is so that we get to test out the build system even if certain
configurations don't apply on your machine.  I don't know if the buildbot
we're using supports this yet, but it is certainly something it should
allow.

Just to check again, we support which versions of ghc at the moment?  I
believe it is 6.4, 6.6 and 6.8?

This set of patches would have been rejected even if I were eager to get
> cabal support into darcs, due to its unreadable nature.  You need to pay
> attention to what changes you're recording as you record patches, and you
> need to avoid including unrelated changes in patches.  It also really
> helps
> to send separate patches in separate bundles whenever possible.


I was going to comment on the whitespace stripping too, but it looks like
you beat me to the send.  I think we should do this sort of thing close to a
tag if we're going to do it, since a patch doing that will not commute very
happily with other patches.  On the other hand, I've never minded extra
whitespace at the end of lines and because of that I think it's a lot of
work and hassle for no gain.  I'd rather we didn't take time to strip out
all the extra spaces, I'm just saying that if it does happen, I think it's
best to synchronize with a tag.

What follows are just comments I have in case Gwern re-sends the amended
patches.

I was uneasy about OPTIONS_GHC, but it does appear that 6.4 supports this,
so as long as I'm right about the supported versions of GHC, then I think
it's fine.  We can only use GHC these days, and we're more tied to GHC than
ever before, but maybe someday some other Haskell compilers will support
enough of the GHC extensions to be a real possibility.

20080416205316: *Prefs.lhs: fmt*

What exactly is this patch doing?

It seems unrelated to the change at hand.  If that's the case then it's
better to send it in a separate patch bundle.  If it is related, it would be
nice to know why (I couldn't figure out why just from reading the hunks).

20080417015657: *darcs.cabal: add optimizations

***
>
> +-- We need optimizations and warnings, irregardless of what Hackage says
>
> Interesting, I was going to say "irregardless" isn't a word, but
dictionary.com has it and just says it's non-standard.  I learn something
new everyday...


20080417022454: *stringify.hs: oops, that version didn't actually work.
scrap fancy lines/unlines solution

*This is probably a good place to use an amend-record, YMMV.

Since you'll need to resend this bundle, I'm not going to apply it yet to
see if it builds here.

Thanks,
Jason

PS I used darcswatch to review these patches and I find it so much nicer
than reviewing the patch via gmail.  I don't think the darcswatch author is
ready to announce anything yet, but darcswatch is so cool and I'm very
excited about it.
**
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osuosl.org/pipermail/darcs-devel/attachments/20080417/66dca23c/attachment-0001.htm 


More information about the darcs-devel mailing list