Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#24980 closed enhancement (fixed)

Use Matchdep module to load installed Grunt plugins

Reported by: jorbin's profile jorbin Owned by: koopersmith's profile koopersmith
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.7
Component: General Keywords: has-patch
Focuses: Cc:

Description

This was originally reported by @kadamwhite over on github for the temp install. His quote is:

"Calling loadNpmTasks for each Grunt plugin installed is duplicative, so Tyler Kellen created a matchdep package to allow Grunt to piggyback on the list of Grunt plugins saved in package.json. Using this module reduces boilerplate and makes it more obvious the package.json is the canonical list of installed plugins. Tyler's a Grunt team member and Ben's endorsed the syntax, so it's an officially-sanctioned approach.

@aaronjorbin alerted me to this syntax, and it's been a huge help wherever I use Grunt. My devil's advocate response to this PR would be concern about adding yet another external module dependency vs using the more-verbose-but-honestly-what-are-ten-lines-among-friends Grunt-native syntax, but I think the clarity could be helpful."

Attachments (3)

matchdep.diff (1.1 KB) - added by jorbin 11 years ago.
matchdep2.diff (1.7 KB) - added by jorbin 11 years ago.
I think I fixed the tabs vs. spaces issue
24980.patch (1.4 KB) - added by kadamwhite 11 years ago.
Adding updated patch to account for the addition of QUnit

Download all attachments as: .zip

Change History (12)

@jorbin
11 years ago

#1 @nacin
11 years ago

  • Milestone changed from Awaiting Review to 3.7
  • Owner set to koopersmith
  • Status changed from new to assigned

@jorbin
11 years ago

I think I fixed the tabs vs. spaces issue

#2 @kadamwhite
11 years ago

  • Cc kadamwhite added

#3 @kadamwhite
11 years ago

As I've been thinking about this, my own response to my devil's advocate challenge would be that since matchdep has come directly out of the Grunt team, it's pretty canonical. Ben regards the long list of task initialization method calls to be "pretty ugly," so doing things in a cleaner fashion here would be nice. +1.

Last edited 11 years ago by kadamwhite (previous) (diff)

#4 @kadamwhite
11 years ago

This would also help make enhancing our Grunt processes easier, such as adding grunt-contrib-qunit as in #25096

#5 follow-up: @claudiosanches
11 years ago

Maybe it would be better to use the load-grunt-tasks: https://npmjs.org/package/load-grunt-tasks

#6 in reply to: ↑ 5 ; follow-up: @jorbin
11 years ago

Replying to claudiosanches:

Maybe it would be better to use the load-grunt-tasks: https://npmjs.org/package/load-grunt-tasks

What benefit does that have over matchdep?

#7 in reply to: ↑ 6 @kadamwhite
11 years ago

Replying to jorbin:

What benefit does that have over matchdep?

I don't see load-grunt-tasks to have any significant benefit over matchdep -- Sindre's great, but matchdep feels more robust (has tests, etc) and I like that it keeps the overall syntax closer to Grunt's out-of-the-box default.

@kadamwhite
11 years ago

Adding updated patch to account for the addition of QUnit

#8 @nacin
11 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 25243:

Use matchdep for Grunt tasks.

props kadamwhite.
fixes #24980.

#9 @SergeyBiryukov
10 years ago

  • Version changed from trunk to 3.7
Note: See TracTickets for help on using tickets.