WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 8 months ago

Last modified 5 months ago

#24980 closed enhancement (fixed)

Use Matchdep module to load installed Grunt plugins

Reported by: jorbin Owned by: 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 9 months ago.
matchdep2.diff (1.7 KB) - added by jorbin 9 months ago.
I think I fixed the tabs vs. spaces issue
24980.patch (1.4 KB) - added by kadamwhite 8 months ago.
Adding updated patch to account for the addition of QUnit

Download all attachments as: .zip

Change History (12)

jorbin9 months ago

comment:1 nacin9 months ago

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

jorbin9 months ago

I think I fixed the tabs vs. spaces issue

comment:2 kadamwhite9 months ago

  • Cc kadamwhite added

comment:3 kadamwhite9 months 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 9 months ago by kadamwhite (previous) (diff)

comment:4 kadamwhite8 months ago

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

comment:5 follow-up: claudiosanches8 months ago

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

comment:6 in reply to: ↑ 5 ; follow-up: jorbin8 months 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?

comment:7 in reply to: ↑ 6 kadamwhite8 months 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.

kadamwhite8 months ago

Adding updated patch to account for the addition of QUnit

comment:8 nacin8 months ago

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

In 25243:

Use matchdep for Grunt tasks.

props kadamwhite.
fixes #24980.

comment:9 SergeyBiryukov5 months ago

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