Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: Update version flags #27237

Merged
merged 2 commits into from
Mar 20, 2020
Merged

build: Update version flags #27237

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ac141d
build: Restore --version flag support, update new flag name
mdmower Mar 14, 2020
b0ab4f9
Revisions based on review
mdmower Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions build-system/compile/internal-version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ const argv = require('minimist')(process.argv.slice(2));
const {gitCommitFormattedTime} = require('../common/git');

function getVersion() {
if (argv.rtv_version) {
return String(argv.rtv_version);
// Flag --version has been supported since 2016, so is still supported for
// backwards compatibility. --release_version is the documented flag.
rsimha marked this conversation as resolved.
Show resolved Hide resolved
if (argv.release_version || argv.version) {
return String(argv.release_version || argv.version);
} else {
// Generate a consistent version number by using the commit* time of the
// latest commit on the active branch as the twelve digits. The last,
Expand Down
9 changes: 6 additions & 3 deletions build-system/tasks/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,9 @@ build.flags = {
noextensions: ' Builds with no extensions.',
core_runtime_only: ' Builds only the core runtime.',
coverage: ' Adds code coverage instrumentation to JS files using istanbul.',
rtv_version: ' Overrides the version number written to AMP_CONFIG',
release_version: ' Overrides the version written to AMP_CONFIG',
};
build.undocumented_flags = ['version'];

watch.description = 'Watches for changes in files, re-builds when detected';
watch.flags = {
Expand All @@ -143,8 +144,9 @@ watch.flags = {
' Watches and builds only the extensions from the listed AMP(s).',
noextensions: ' Watches and builds with no extensions.',
core_runtime_only: ' Watches and builds only the core runtime.',
rtv_version: ' Overrides the version number written to AMP_CONFIG',
release_version: ' Overrides the version written to AMP_CONFIG',
};
watch.undocumented_flags = ['version'];

defaultTask.description =
'Starts the dev server and lazily builds JS and extensions when requested';
Expand All @@ -154,5 +156,6 @@ defaultTask.flags = {
extensions_from:
' Watches and builds only the extensions from the listed AMP(s).',
noextensions: ' Watches and builds with no extensions.',
rtv_version: ' Overrides the version number written to AMP_CONFIG',
release_version: ' Overrides the version written to AMP_CONFIG',
};
defaultTask.undocumented_flags = ['version'];
3 changes: 2 additions & 1 deletion build-system/tasks/dist.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,5 +452,6 @@ dist.flags = {
" Doesn't use nailgun to invoke closure compiler (much slower)",
type: ' Points sourcemap to fetch files from the correct GitHub tag',
esm: ' Does not transpile down to ES5',
rtv_version: ' Override the version number written to AMP_CONFIG',
release_version: ' Override the version written to AMP_CONFIG',
};
dist.undocumented_flags = ['version'];
8 changes: 4 additions & 4 deletions contributing/TESTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ Before running these commands, make sure you have Node.js, yarn, and Gulp instal
| `gulp --extensions=minimal_set` | Runs "watch" and "serve", after building the extensions needed to load `article.amp.html`. |
| `gulp --extensions_from=examples/foo.amp.html` | Runs "watch" and "serve", after building only extensions from the listed examples. |
| `gulp --noextensions` | Runs "watch" and "serve" without building any extensions. |
| `gulp --rtv_version=<rtv_version>` | Runs "watch" and "serve". Overrides the version nubmer written to the AMP_CONFIG. |
| `gulp --release_version=<release_version>` | Runs "watch" and "serve". Overrides the version written to the AMP_CONFIG. |
| `gulp dist` | Builds production binaries and applies AMP_CONFIG to runtime files. |
| `gulp dist --noconfig` | Builds production binaries without applying AMP_CONFIG to runtime files. |
| `gulp dist --extensions=amp-foo,amp-bar` | Builds production binaries, with only the listed extensions. |
Expand All @@ -58,7 +58,7 @@ Before running these commands, make sure you have Node.js, yarn, and Gulp instal
| `gulp dist --core_runtime_only` | Builds production binary for just the core runtime. |
| `gulp dist --fortesting` | Builds production binaries for local testing. (Allows use cases like ads, tweets, etc. to work with minified sources. Overrides `TESTING_HOST` if specified. Uses the production `AMP_CONFIG` by default.) |
| `gulp dist --fortesting --config=<config>` | Builds production binaries for local testing, with the specified `AMP_CONFIG`. `config` can be `prod` or `canary`. (Defaults to `prod`.) |
| `gulp dist --rtv_version=<rtv_version>` | Builds production binaries and overrides the version nubmer written to the AMP_CONFIG. |
| `gulp dist --release_version=<release_version>` | Builds production binaries and overrides the version written to the AMP_CONFIG. |
| `gulp lint` | Validates JS files against the ESLint linter. |
| `gulp lint --watch` | Watches for changes in files, and validates against the ESLint linter. |
| `gulp lint --fix` | Fixes simple lint warnings/errors automatically. |
Expand All @@ -75,7 +75,7 @@ Before running these commands, make sure you have Node.js, yarn, and Gulp instal
| `gulp build --noextensions` | Builds the AMP library with no extensions. |
| `gulp build --core_runtime_only` | Builds only the core runtime of the AMP library. |
| `gulp build --fortesting` | Builds the AMP library and sets the `test` field in `AMP_CONFIG` to `true`. |
| `gulp build --rtv_version=<rtv_version>` | Builds the AMP library with the specified version number. |
| `gulp build --release_version=<release_version>` | Builds the AMP library with the specified version. |
| `gulp check-links --files=<files-path-glob>` | Reports dead links in `.md` files. |
| `gulp check-links --local_changes` | Reports dead links in `.md` files changed in the local branch. |
| `gulp clean` | Removes build output. |
Expand All @@ -87,7 +87,7 @@ Before running these commands, make sure you have Node.js, yarn, and Gulp instal
| `gulp watch --extensions_from=examples/foo.amp.html` | Watches for changes in files, re-builds only the extensions needed to load the listed examples. |
| `gulp watch --noextensions` | Watches for changes in files, re-builds with no extensions. |
| `gulp watch --core_runtime_only` | Watches for changes in the core runtime, re-builds. |
| `gulp watch --rtv_version=<rtv_version>` | Watches for changes in files, rebuilds. Overrides the version nubmer written to the AMP_CONFIG. |
| `gulp watch --release_version=<release_version>` | Watches for changes in files, rebuilds. Overrides the version written to the AMP_CONFIG. |
| `gulp pr-check` | Runs all the Travis CI checks locally. |
| `gulp pr-check --nobuild` | Runs all the Travis CI checks locally, but skips the `gulp build` step. |
| `gulp pr-check --files=<test-files-path-glob>` | Runs all the Travis CI checks locally, and restricts tests to the files provided. |
Expand Down
3 changes: 3 additions & 0 deletions gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ function checkFlags(name, taskFunc) {
return; // This isn't the task being run.
}
const validFlags = taskFunc.flags ? Object.keys(taskFunc.flags) : [];
if (Array.isArray(taskFunc.undocumented_flags)) {
Array.prototype.push.apply(validFlags, taskFunc.undocumented_flags);
}
rsimha marked this conversation as resolved.
Show resolved Hide resolved
const usedFlags = Object.keys(argv).slice(1); // Skip the '_' argument
const invalidFlags = [];
usedFlags.forEach(flag => {
Expand Down