🏗 Clean up a few things in gulpfile.js

[rsimha]

We use gulp-help in gulpfile.js to provide developers with detailed help for how to use various tasks. This PR was necessitated because the gulp4 branch of gulp-help hasn't been published to npm, and directly using the github branch in package.json can result in rate limiting during the package installation step in CI builds.

amphtml/package.json

Line 118 in f6a651b

"gulp-help": "chmontgomery/gulp-help#gulp4",

PR highlights:

• Remove all usage of gulp-help
• Replace gulp help with the native gulp --tasks
• Remove the watch task from gulpfile.js (was deprecated a year ago)
• Remove old warning for gulp-util (used to accompany gulp 3)
• Fix help text indentation in a couple places

Coming up:

• Clean up descriptions (fix typos, shorten really long ones)

[estherkim]

Looks like adding gulp help 4 to third_party increased the bundle size which we probably don't want to do.

Maybe the lack of an npm release and support from the developer are signs that we should consider alternatives to gulp-help.

[rsimha]

Looks like adding gulp help 4 to third_party increased the bundle size which we probably don't want to do.

Files in third_party do not affect the bundle-size unless they are explicitly listed here:

amphtml/build-system/compile/sources.js

Lines 23 to 39 in 1ff2dfb

	const COMMON_GLOBS = [
	'third_party/amp-toolbox-cache-url/**/*.js',
	'third_party/caja/html-sanitizer.js',
	'third_party/closure-library/sha384-generated.js',
	'third_party/css-escape/css-escape.js',
	'third_party/d3/**/*.js',
	'third_party/fuzzysearch/index.js',
	'third_party/inputmask/**/*.js',
	'third_party/mustache/**/*.js',
	'third_party/react-dates/bundle.js',
	'third_party/resize-observer-polyfill/ResizeObserver.install.js',
	'third_party/set-dom/set-dom.js',
	'third_party/subscriptions-project/*.js',
	'third_party/timeagojs/**/*.js',
	'third_party/vega/**/*.js',
	'third_party/webcomponentsjs/ShadowCSS.js',
	'third_party/zuho/**/*.js',

I suspect the bundle-size failure is a red herring due to other commits that landed on master in the meantime.

I've rebased this PR on master to make sure. Edit: Rebase shows no size increase.

Maybe the lack of an npm release and support from the developer are signs that we should consider alternatives to gulp-help.

Unfortunately, there aren't any known alternatives that do nearly as good a job with listing all gulp tasks, flags, and descriptions. (Unless you can find one that I missed?)

I could remove gulp-help altogether, but that will hamper the usability of our tools, given all the places where we ask developers to run gulp help for more details.

[estherkim]

So gulp-cli has this feature already, see https://github.com/gulpjs/gulp-cli#flags

Unfortunately it doesn't work with our setup, something about the default task overriding a gulp -T command, but I messed around a little to see what it might look like...

% gulp lint -T
[16:28:13] Tasks for ~/src/ampproject/amphtml/gulpfile.js
[16:28:13] ├── help                        Display this help text.
[16:28:13] ├─┬ h
[16:28:13] │ └─┬ <series>
[16:28:13] │   └── help
[16:28:13] ├── default                     Starts the dev server, lazily builds JS and extensions when requested, and watches them for changes
[16:28:13] │   closure_concurrency         …  Sets the number of concurrent invocations of closure
[16:28:13] │   compiled                    …  Compiles and serves minified binaries
[16:28:13] │   config                      …  Sets the runtime's AMP_CONFIG to one of "prod" or "canary"
[16:28:13] │   define_experiment_constant  …  Builds runtime with the EXPERIMENT constant set to true
[16:28:13] │   extensions                  …  Pre-builds the given extensions, lazily builds the rest.
[16:28:13] │   extensions_from             …  Pre-builds the extensions used by the provided example page.
[16:28:13] │   fortesting                  …  Compiles production binaries for local testing
[16:28:13] │   full_sourcemaps             …  Includes source code content in sourcemaps
[16:28:13] │   host                        …  Host to serve the project on. localhost by default.
[16:28:13] │   https                       … Use https server. http by default.
[16:28:13] │   noconfig                    …  Compiles production binaries without applying AMP_CONFIG
[16:28:13] │   port                        …  Port to serve the project on. 8000 by default.
[16:28:13] │   pretty_print                …  Outputs compiled code with whitespace. Great for debugging production code.
[16:28:13] │   pseudo_names                …  Compiles with readable names. Great for profiling and debugging production code.
[16:28:13] │   version_override            …  Overrides the version written to AMP_CONFIG
[16:28:13] ├── a4a                         Runs a4a tests
[16:28:13] │   chrome_canary               …  Runs tests on Chrome Canary
[16:28:13] │   chrome_flags                …  Uses the given flags to launch Chrome
[16:28:13] │   files                       …  Runs tests for specific files
[16:28:13] │   firefox                     …  Runs tests on Firefox
[16:28:13] │   grep                        …  Runs tests that match the pattern
[16:28:13] │   headless                    …  Run tests in a headless Chrome window
[16:28:13] │   ie                          …  Runs tests on IE
[16:28:13] │   nobuild                     …  Skips build step
[16:28:13] │   nohelp                      …  Silence help messages that are printed prior to test run
[16:28:13] │   safari                      …  Runs tests on Safari
[16:28:13] │   testnames                   …  Lists the name of each test being run
[16:28:13] │   verbose                     …  With logging enabled
[16:28:13] │   watch                       …  Watches for changes in files, runs corresponding test(s)

Since this PR is to prevent CI build rate limiting, I'm ok with this PR and adding a TODO to remove gulp-help if the above suffices as a replacement.

[rsimha]

So gulp-cli has this feature already, see gulpjs/gulp-cli#flags

Since this PR is to prevent CI build rate limiting, I'm ok with this PR and adding a TODO to remove gulp-help if the above suffices as a replacement.

Oooh, I'm glad you reviewed and provided pushback on this! The alternative does look promising. Since the rate limiting appears to be intermittent, I'm going to attempt to fix this without the third_party changes. (If the fix isn't straightforward, or if there are more broken builds, I'll merge this and send you a follow up.)

[rsimha]

Unfortunately it doesn't work with our setup, something about the default task overriding a gulp -T command, but I messed around a little to see what it might look like...

Managed to fully remove gulp-help and get gulp --tasks to work. All that was missing was to document the --tasks flag in default-task.js.

Ready for one last review.

[estherkim]

Amazing 💯 💯 💯

[rsimha]

For posterity, here's a really zoomed-out screenshot of gulp --tasks. 95% as pretty as gulp-help, but 9.5x better because it's natively supported and doesn't involve the old package.json hack.

A+++ review from @estherkim, would request again 😃