Skip to content

🏗 Fixes to check-package-manager.js#24060

Merged
rsimha merged 1 commit intoampproject:masterfrom
rsimha:2019-08-19-CheckPackageManager
Aug 20, 2019
Merged

🏗 Fixes to check-package-manager.js#24060
rsimha merged 1 commit intoampproject:masterfrom
rsimha:2019-08-19-CheckPackageManager

Conversation

@rsimha
Copy link
Copy Markdown
Contributor

@rsimha rsimha commented Aug 19, 2019

We use check-package-manager.js to run several checks on the AMP toolchain (node, yarn, gulp, python) during development.

A couple of the checks for gulp-cli assume that the tool was installed using yarn global add, as our dev docs currently recommend:

  • The gulp executable path is valid
  • The gulp-cli command line tool is installed

The resulting warnings can be annoying to those who are using alternatives like npx, Lando, or Volta.

This PR does the following:

  • Changes the wrongGulp check to print a warning for just well-known bad paths [/usr[/local]]/[s]bin/, thereby allowing for all other valid paths
  • Removes the check for a global gulp-cli, since it's annoying to those who use alternatives

Addresses #23213 (comment)
Follow up to #22459 and #22479

@rsimha rsimha requested a review from westonruter August 19, 2019 23:04
@rsimha rsimha self-assigned this Aug 19, 2019
@rsimha rsimha merged commit c095fcf into ampproject:master Aug 20, 2019
@rsimha rsimha deleted the 2019-08-19-CheckPackageManager branch August 20, 2019 16:49
westonruter added a commit to westonruter/amphtml that referenced this pull request Aug 20, 2019
* 'master' of github.com:ampproject/amphtml: (32 commits)
  ✨ Make tweet id a bindable attribute (ampproject#23953)
  🏗 Update Local AMP extension to allow custom base URLs (ampproject#24029)
  🏗 Improve serving from non-localhost host (ampproject#24066)
  Preventing half pixels. (ampproject#24050)
  Update callout-vendors.js (ampproject#23218)
  🏗 Fixes to `check-package-manager.js` (ampproject#24060)
  Rename AMP_MODE to __AMP_MODE. (ampproject#24052)
  Story media performance metrics. (ampproject#23962)
  Updating Story amp-sidebar width documentation. (ampproject#23894)
  Fixes race condition in amp-video-iframe (ampproject#24033)
  Rename ampExtendedElements to __AMP_EXTENDED_ELEMENTS (ampproject#24056)
  🏗🚮 Enable property inlining (ampproject#24053)
  ✨amp-ads: Added optional params for Directadvert network (ampproject#23724)
  <amp-experiment> style mutation fix and improvment (ampproject#23669)
  🐛 Allow http protocol for noscript > img fallbacks for parity with amp-img (ampproject#21686)
  🏗 Refactor transform-log-asserts (ampproject#24028)
  Automatically preconnect to source origins on page loads. (ampproject#24045)
  Support visibility API in the ampdoc (ampproject#23799)
  Amphtml visual tests should use relative path against root (ampproject#24042)
  FIX: check all fields' dirtiness on AMP form init (ampproject#23978)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants