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

options by task #75

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

options by task #75

wants to merge 4 commits into from

Conversation

mattkime
Copy link

No description provided.

@mattkime
Copy link
Author

mattkime commented Apr 9, 2015

i've spent some time trying to figure out why this is failing for node 0.8.x. works fine on my box but would like to get to the bottom of it.

@mattkime
Copy link
Author

@tschaub I'm considering forking grunt-newer due to lack of activity solving cases where file filtering isn't wanted or isn't directly possible. I'd rather work within the existing grunt-newer project but there's been little in the way of feedback.

@tschaub
Copy link
Owner

tschaub commented Apr 24, 2015

@mattkime I appreciate your effort to try to make this more usable. However, I don't think this change makes a significant improvement. There are now two ways to provide an override, and the second only saves a bit of typing.

In addition, supporting options named like other tasks has two potential issues. First, any option added in the future (regular options for the newer task) could collide with the names of tasks for which people had previously been trying to provide a custom override. For example, your change would support this:

  newer: {
    options: {
      foo: {
        override: overrideForFoo
      }
    }
  }

And in the future, if the newer task supported a foo option, it would break this use without knowing it.

The second issue is that people might wrongly assume that they could provide other per-task options in this same way. Why is the override option special? If there are two ways to provide the override option, it might be surprising that all options don't work the same way.

I know that this is a bit pedantic (especially since there are only two options for the task). But I think a general fault of Grunt is that it provides too many ways to do the same thing (e.g. specify src and dest files), and task handling is sloppy and awkward because of it.

So, I think it would be great to make it easier to make grunt-newer work with LESS imports (or other tasks that use source files not specified in the task directly). I think instead of supporting a different way to specify the override option, it would be more useful to support something like this:

  newer: {
    options: {
      override: overrides({
        less: overrides.less(),
        someOtherTaskName: overrides.someOtherOverride(whateverConfig)
      })
    }
  }

And the grunt-newer package could include a set of common overrides (e.g. one for checking LESS imports).

@mattkime
Copy link
Author

@tschaub i'm a bit confused about how to apply the overrides option to the simplest of use cases - if one file changes, lets include all files. (yeah, a bit off topic)

i agree that grunt has its weaknesses, but I've yet to find an alternative that makes switching worthwhile. (gulp does have its virtues, not sure if they're sufficient to switch)

i see your point about task vs newer option namespace collisions. after chewing on this for a while i think it might be better to throw caution to the wind and put the newer task config directly inside the task or target.

uglify: {
    options: {
        newer: { // for all uglify targets
            override: function(){}
        }
    },
    build: {
        newer: { // target specific config ... hm, would this ever be useful? maybe this is a task level thing
            override: function2(){}
        },
        src: 'src/<%= pkg.name %>.js',
        dest: 'build/<%= pkg.name %>.min.js'
    }
}

yes, yes, there may be collisions. but this is how i want to express what needs to happen. perhaps '_newer' instead of 'newer'? or make it configurable via newer task options?

restating the problem - how can we supply config info for a task that modifies other tasks? and can we do it in such a way that allows for additional options?

mattkime and others added 2 commits October 29, 2015 16:54
license update / pull from grunt-newr
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.

2 participants