-
Notifications
You must be signed in to change notification settings - Fork 696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rename all-packages
to package *
#5053
Conversation
Using `package *` for all packages is in line with `package foo` for and does not introduce a new keyword to remember.
@@ -81,7 +81,7 @@ import Distribution.Simple.Command | |||
, OptionField, option, reqArg' ) | |||
|
|||
import qualified Data.Map as Map | |||
|
|||
import Debug.Trace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kill me
Good thing we fixed this before release, otherwise I'd tell you that you have to support the old syntax too XD |
There needs to be a test. |
Right, that's why I tried to wedge it in as early as possible.
There are already tests from #4972 in the testsuite (https://github.com/haskell/cabal/pull/4972/files#diff-a047033a96ad8af1f66c9061abc0aca5), what kind of additional tests do you envision? |
I looked in the diff and didn't see any test files which got changed from |
Yep, @dcoutts never added them and we decided to merge that PR anyway lest it bitrot. |
The current existing tests are only the quick check round trip tests. Which were quite helpful when implementing this change as. What kind of additional tests do we need? |
The linux-7.8.4 failure looks like Travis fails to find and install ghc and cabal via apt-get. Maybe it’s dead? |
This happens sometimes, rerunning the build usually helps. |
@23Skidoo I can't restart the Downstream Travis / haskell-pushbot jobs, who can do that? |
@23Skidoo so should I just go ahead and merge this one? |
I restarted all of the Travis builds by closing/reopening the PR. |
@ezyang it's the
Travis has the following yellow warning attached as well:
|
The downstream Travis builds are triggered by the regular Travis build, so if the regular one restarts, the downstream ones restart too. The real problem seems to be that our Travis queue is completely hosed right now. Since Travis claims there's no outage right now, I'm just going to cancel all ongoing jobs and see if that unsticks it. |
:( |
Closed & reopened to restart CI. OOM issue is resolved in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks OK, would be nice if you could add a test while you're at it.
This is finally green, merging. |
Merged, thanks! |
Using
package *
for all packages is in line withpackage foo
forand does not introduce a new keyword to remember.
Please include the following checklist in your PR:
[ci skip]
is used to avoid triggering the build bots.Please also shortly describe how you tested your change. Bonus points for added tests!