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

Fixed grunt newer failing to restore configuration #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NullSoldier
Copy link

When grunt newer runs a task that fails, it will not restore the configuration leading to numerous silent bugs, especially when running newer on things like watch. Using override will solve this problem as we have no clean way to run post tasks once the main task has failed. This is basically re-creating try finally using force.

If you like this approach to solving the problem, I can create tests so we can get this merged in.

I got the idea for a force task from the comment here, gruntjs/grunt#810 (comment)

@NullSoldier
Copy link
Author

I've been diving into the grunt source code, and i think there may be a way to run a task and know when it finishes by using grunt.task directly. I would like to look into it then open this PR again once I know if there is a better way or not.

@NullSoldier NullSoldier reopened this May 20, 2015
@diwu1989
Copy link

the force option is to enable the --force behavior built into grunt

from the grunt command line help

      --force, -f  A way to force your way past warnings. Want a suggestion?
                   Don't use this option, fix your code.

grunt.registerTask("newer-force", function(set) {
if (set === "on") {
previous_force_state = grunt.option("force");
grunt.option("force", true);

Choose a reason for hiding this comment

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

double quote vs single quote for strings

@diwu1989 diwu1989 force-pushed the fix-restore branch 4 times, most recently from 1a6aee6 to 3b9b7ac Compare June 15, 2015 06:50
@diwu1989
Copy link

pull request cleaned up @tschaub wanna review this?

@tschaub
Copy link
Owner

tschaub commented Jun 15, 2015

Can you provide an example that demonstrates the problem (and show how this change addresses it)? It's not clear looking at the code alone. An example repo or a gist with enough code to reproduce the issue would be great.

@terite
Copy link

terite commented May 5, 2016

It looks like @guncha uploaded repro steps - guncha/grunt-newer-conf-corruption@1deb15b

@tschaub , are you able to reproduce using that test case?

Our team had to fork grunt-newer a while ago to fix this issue. We'd really like to delete our fork, but this is a blocking issue for us.

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

4 participants