Skip to content
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

Additional warnings for 'cabal run' #2510

Merged
merged 3 commits into from Apr 1, 2015
Merged

Conversation

lspitzner
Copy link
Collaborator

see #2507

just additional warnings; did not implement anything in the direction of running test-suites or benchmarks (even thought that might be nice).

Just exe -> return (True, exe, xs)
where
enabledExes = filter (buildable . buildInfo) (executables pkg_descr)
printMaybeWarningWithAdditional :: String -> IO ()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's simpler to just make this Maybe String and then use e.g. notice $ concat (maybeToList mbWarn ++ [myWarn]).

@lspitzner
Copy link
Collaborator Author

printMaybeWarningWithAdditional does not print anything if the warning-Maybe is Nothing, which you seem to have misunderstood in both your comments, or am i wrong?

@23Skidoo
Copy link
Member

I guess the name tripped me up, perhaps a better one would be maybePrintWarning.... The first comment still applies, though. I think it's better to make maybeWarning a Maybe String and then do something like maybe (return ()) (notice verbosity) ((++ myMsg) <$> maybeWarning) in the code that uses it.

@lspitzner
Copy link
Collaborator Author

style question: traverse (notice normal) maybeWarning or notice normal traverse maybeWarning

@lspitzner
Copy link
Collaborator Author

(or was there a reason you preferred maybe?)

@23Skidoo
Copy link
Member

style question: traverse (notice normal) maybeWarning or notice normal traverse maybeWarning

Whichever you prefer. Shouldn't it be traverse_, though?

(or was there a reason you preferred maybe?)

Nope, no problem with traverse.

@23Skidoo 23Skidoo changed the title Runcomponent Additional warnings for 'cabal run' Mar 31, 2015
@lspitzner
Copy link
Collaborator Author

.. yes, should be traverse_.

(and i thought i had -Wall configured..)

++ " the default executable."
-- if there is a warning, print it together
-- with the addition.
notice normal `traverse_` fmap (++addition)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, there's a warn function for printing warnings.

23Skidoo added a commit that referenced this pull request Apr 1, 2015
Additional warnings for 'cabal run'
@23Skidoo 23Skidoo merged commit c940d16 into haskell:master Apr 1, 2015
@23Skidoo
Copy link
Member

23Skidoo commented Apr 1, 2015

Merged, thanks!

@lspitzner lspitzner deleted the runcomponent branch October 22, 2015 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants