-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Should tasks with errors warn or fail? #1163
Comments
cc @shama |
Fail as you have described but each contrib plugin has a
Provide an API that lets users configure how grunt deals with warning/errors. Defaults to the existing behavior but a user can override what should happen on error or warning; globally and per task. |
Pretty much what @shama said. |
@shama I'd love to chat more about this, are you around at all tomorrow? |
As mentioned in #810, repeated here since no one mentions the exit code: Whatever the solution here, I'd like you to consider a related, pretty important aspect: The exit code. Currently, setting the force option on tasks that support it will cause grunt to exit with a zero status code, even when one or more forced tasks had errors. My usecase is pretty simple: Run all tasks, but exit with non-zero status code if anything failed. That way I'd get the most out of Travis builds, where a trailing comma can be printed as an error in the jshint task and causes the build to fail, but it would still run the unit tests. |
👍 to what @jzaefferer says. |
@jzaefferer what happens when jshint fails because of a syntactic error that prevents the tests from running, or causes them to hang in some way? |
@jzaefferer this already does this? See this travis build: JSCS errors, rest of the tests run. Final build fails. https://travis-ci.org/mozilla/fxa-content-server/builds/27542793#L1386 |
@vladikoff there's way too much output there to tell what's going on. Last I tried the global
That should probably still abort the runner with a non-zero exit code, independent of the global I think it makes sense to have a I don't think there should be a difference between errors and warnings. Allowing warnings that don't cause the build to fail leads to the terrible state where running a build outputs tons of warnings that everyone ignores. |
@jzaefferer what kind of message do you output when someone uses a deprecated feature or has configured the plugin in a sub-optimal way? |
I don't know what "sub-optimal" means here, but throwing an error with a message and stacktrace and aborting execution sounds sane for that kind of issue. |
Is there a way to supress errors or warnings of a task till all other tasks have completed and throw error in the end(collated) ? |
We've seen this issue pop up a bunch of times in the contrib plugins. Some people want a failing task to warn, so that following tasks continue to run. Other people want a failing task to error, so that following tasks don't run.
Currently, the only way to allow the user control over this is to have failing fail, causing any following tasks to not run, which can then be overridden globally with the
--force
CLI option. Is this how we're currently handling this in contrib?This seems to be the ideal behavior:
Examples:
I'd like to discuss these two questions:
The text was updated successfully, but these errors were encountered: