Clarify --add-prefetch build flag
#510
Conversation
| @@ -34,7 +34,7 @@ const buildDirectory = 'build/'; | |||
|
|
|||
| export interface BuildOptions { | |||
| swPrecacheConfig?: string; | |||
| insertDependencyLinks?: boolean; | |||
| prefetchDepedencies?: boolean; | |||
There was a problem hiding this comment.
"insert" seemed meaningful here, in that the effect of this flag is to insert prefetch link tags, the effect isn't to do prefetching.
There was a problem hiding this comment.
Good point, changed naming to addPrefetchLinks to be in line with the cli flag.
| description: 'Flatten dependency tree downloads by inserting ' + | ||
| 'additional `<link rel="prefetch">` tags into ' + | ||
| 'entrypoint and `<link rel="import">` tags into fragments and shell.' | ||
| description: 'Add dependency tree prefetching by inserting ' + |
There was a problem hiding this comment.
I'd remove "tree", we haven't used the term "dependency tree" before. Maybe:
"Enable dependency prefetching by inserting..."
| @@ -45,11 +45,12 @@ export class BuildCommand implements Command { | |||
| 'used for service worker generation.' | |||
| }, | |||
| { | |||
| name: 'insert-dependency-links', | |||
| name: 'add-prefetch', | |||
There was a problem hiding this comment.
similar to the above comment, I think "insert" is meaningful here. Maybe insert-prefetch-links?
There was a problem hiding this comment.
I prefer the shorter add-prefetch when it comes to command line args. And insert always felt like it was inserting something new into my application while add feels like it's adding a new feature/ability to my application. So I prefer as-is, but happy to make the change if you feel strongly.
There was a problem hiding this comment.
I'm not really concerned about long flag names for things that'll hopefully be in a config file, if they're more descriptive. I think insert-prefetch-links is the most clear name.
There was a problem hiding this comment.
fine by me, I'll make the change
|
@justinfagnani name changed, PTAL |
* rename confusing --insert-dependency-links flag to --prefetch-dependencies * rename prefetchDepedencies to addPrefetchLinks * rename to insert-prefetch-links
* Convert build to single build stream, add bundled flag * Clarify `--add-prefetch` build flag (#510) * rename confusing --insert-dependency-links flag to --prefetch-dependencies * rename prefetchDepedencies to addPrefetchLinks * rename to insert-prefetch-links * respond to review comments
* Convert build to single build stream (#509) * Convert build to single build stream, add bundled flag * Clarify `--add-prefetch` build flag (#510) * rename confusing --insert-dependency-links flag to --prefetch-dependencies * rename prefetchDepedencies to addPrefetchLinks * rename to insert-prefetch-links * respond to review comments * Cleanup how optimize streams work, in preparation for babel stream (#524) * Cleanup how optimize streams work, in preparation for babel stream * death to lets * redeisn optimize interface * pull stream piping into a helper function * rollback cli dash-case naming * remove unused gulpif * update buildoptions to match planned polymer.json options * add babel to build stream, replace uglify with babili es2015 minifier (#525) * Cleanup how optimize streams work, in preparation for babel stream * add babel to build stream * death to lets * redeisn optimize interface * pull stream piping into a helper function * rollback cli dash-case naming * replace uglify with babili * fix bad merge conflict * fixup js-minify description * remove unused gulpif * update buildoptions to match planned polymer.json options * remove old comment * let to const * add service worker flag to build command (#534) * add service worker flag to build command * reword flags and descriptions * quick reword again * get rid of skiplibcheck * debug to info * Add support for polymer.json build configurations (#545) * Add support for named build permutations * respond to PR feedback * add comments on build function * update logging to be cleaner * update for new naming rules * update polymer-project-config to v1.2.0 * Update CHANGELOG.md * update polymer-build version, update shrinkwrap
Clarified this build flag since a lot of people (myself included) weren't sure what it was doing. Also confirmed it's still working as intended.
/cc @justinfagnani