- User Since
- Apr 27 2016, 1:39 PM (136 w, 6 d)
Many thanks, @alpmestan! All looks great now.
What a nice little build rule. We could add some more intelligence (see my comments), although what you already have is very useful too.
Where else do you want this to be documented?
This looks great. Thank you for the patch!
Hope that's enough to fix the issue.
Fri, Dec 7
@alpmestan Awesome, thank you!
Thu, Dec 6
@alpmestan Thank you! Looks perfectly reasonable to me.
@harpocrates Thank you for the patch.
@alpmestan Many thanks! Your patch looks great now.
Wed, Dec 5
Tue, Dec 4
@alpmestan Many thanks! In general this looks great, but I left a few comments/questions for you.
Sun, Dec 2
And another thumb-up from me. Thanks @harpocrates!
Wed, Nov 28
Thanks! Sure, let's bump the bounds.
Thanks @alpmestan, I agree with your thoughts.
Tue, Nov 27
Looks good to me!
Mon, Nov 26
Thanks! Yes, that's the right solution for now, but I'm happy to discuss/help with a better design.
Thu, Nov 22
Let me capitalise the title, since Hadrian is a name :-)
@alpmestan Many thanks, this is a very helpful note.
Like @bgamari, I'd like to have a note about wrappers somewhere in the code, as otherwise it's always a matter of googling for hours in order to find out more information about them.
Tue, Nov 20
Thank you! i believe this can now be merged.
Thank you! This looks good to me, although we should remember to merge this commit with Hadrian: prefix, since it's a Hadrian-only commit.
One more thing: we'd like to follow a convention that all Hadrian-only commits are prefixed with Hadrian: . I guess renaming this diff should be sufficient for it to land with the right commit message.
@harpocrates Thank you! The newly added comments and simplifications to the docs rule are very helpful.
Wed, Nov 14
Additionally, I noticed that there Hadrian doesn't generate the short wrapper shell script the make-based system used to produce
Tue, Nov 13
@harpocrates Many thanks! Apologies for taking so long to review. I've left a few minor comments, mostly just trying to make this patch a bit simpler.
Oct 27 2018
@watashi Thank you! Generally this looks good, but if possible I would like to avoid baking in the special case package /= rts (see my comment). Can this be specified in rts.cabal.in instead?
Oct 26 2018
@alpmestan I'm happy this fixes Hadrian, but I feel unqualified to review this patch from the GHC perspective. In my opinion, the way compareByPreference and integer-wired-in interact is questionable and error-prone (both before and after this patch), which should probaby be discussed in a separate GHC ticket.
Oct 25 2018
@alpmestan Great! Is there a way to migrate our AppVeyor and Travis scripts too?
Oct 24 2018
@alpmestan Thank you, this looks good to me. However, have we lost all CI support with the move from GitHub? Can we fix the CI first?