Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

chore(npm): use require.resolve when possible to avoid hard coded mod… #15071

Closed
wants to merge 2 commits into from

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Aug 30, 2016

Cleaned up and rebased version of #13320.

I've tested the affected commands locally, and they all went through.

@Narretz Narretz added this to the 1.5.9 milestone Aug 30, 2016
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

1 similar comment
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@@ -39,7 +39,7 @@ module.exports = function(grunt) {


grunt.registerTask('docs', 'create angular docs', function() {
var gruntProc = shelljs.exec('"node_modules/.bin/gulp" --gulpfile docs/gulpfile.js');
var gruntProc = shelljs.exec('node "' + require.resolve('gulp/bin/gulp') + '" --gulpfile docs/gulpfile.js');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to add node here, otherwise Windows wouldn't execute the command

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather not depend on the internal gulp package structure. How about creating the npm script gulp in package.json:

{
  "scripts": {
    "gulp": "gulp
  }
}

and then npm run gulp -- --gulpfile docs/gulpfile.js should work everywhere.

@Narretz
Copy link
Contributor Author

Narretz commented Sep 2, 2016

@mgol I've updated the PR with your suggestions, can you take a look?

@mgol
Copy link
Member

mgol commented Sep 5, 2016

LGTM.

@Narretz Narretz closed this in 9fbad3c Sep 5, 2016
Narretz added a commit that referenced this pull request Sep 8, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Nov 21, 2016
petebacondarwin pushed a commit that referenced this pull request Nov 23, 2016
petebacondarwin pushed a commit that referenced this pull request Nov 24, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants