Remove the "config change cannot sits in same PR" check (#22942)

* remove * Clean up code * remove 1
ampproject · Jun 26, 2019 · a844c8f · a844c8f
1 parent 3aef004
commit a844c8f
Show file tree

Hide file tree

Showing 11 changed files with 13 additions and 91 deletions.
diff --git a/build-system/pr-check/build-targets.js b/build-system/pr-check/build-targets.js
@@ -162,54 +162,16 @@ const targetMatchers = [
   },
 ];
 
-/**
- * Returns false if flag config files are mixed with any other files.
- * @param {!Set<string>} buildTargets
- * @param {string} fileName
- * @return {boolean}
- */
-function areValidBuildTargets(buildTargets, fileName) {
-  const files = gitDiffNameOnlyMaster();
-  const fileLogPrefix = colors.bold(colors.yellow(`${fileName}:`));
-  if (buildTargets.has('FLAG_CONFIG') && buildTargets.size > 1) {
-    console.log(
-      fileLogPrefix,
-      colors.red('ERROR:'),
-      'Looks like your PR contains',
-      colors.cyan('{prod|canary}-config.json'),
-      'in addition to other files.'
-    );
-    console.log(
-      fileLogPrefix,
-      colors.red('ERROR:'),
-      'AMP config files need to be backward compatible with source code for at',
-      'least two weeks. See https://github.com/ampproject/amphtml/issues/8188.'
-    );
-    const nonFlagConfigFiles = files.filter(
-      file => !file.startsWith('build-system/global-configs/')
-    );
-    console.log(
-      fileLogPrefix,
-      colors.red('ERROR:'),
-      'Please move these files to a separate PR:\n',
-      colors.cyan(nonFlagConfigFiles.join('\n '))
-    );
-    return false;
-  }
-  return true;
-}
-
 /**
  * Populates buildTargets with a set of build targets contained in a PR after
  * making sure they are valid. Used to determine which checks to perform / tests
  * to run during PR builds.
- * @param {!Set<string>} buildTargets
  * @param {string} fileName
  * @return {boolean}
  */
-function determineBuildTargets(buildTargets, fileName = 'build-targets.js') {
+function determineBuildTargets(fileName = 'build-targets.js') {
   const filesChanged = gitDiffNameOnlyMaster();
-  buildTargets.clear;
+  const buildTargets = new Set();
   for (const file of filesChanged) {
     let matched = false;
     targetMatchers.forEach(matcher => {
@@ -231,7 +193,7 @@ function determineBuildTargets(buildTargets, fileName = 'build-targets.js') {
         .join(', ')
     )
   );
-  return areValidBuildTargets(buildTargets, fileName);
+  return buildTargets;
 }
 
 module.exports = {

diff --git a/build-system/pr-check/build.js b/build-system/pr-check/build.js
@@ -52,13 +52,7 @@ function main() {
     uploadBuildOutput(FILENAME);
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    if (!determineBuildTargets(buildTargets, FILENAME)) {
-      stopTimer(FILENAME, FILENAME, startTime);
-      process.exitCode = 1;
-      return;
-    }
-
+    const buildTargets = determineBuildTargets(FILENAME);
     if (
       buildTargets.has('RUNTIME') ||
       buildTargets.has('FLAG_CONFIG') ||

diff --git a/build-system/pr-check/checks.js b/build-system/pr-check/checks.js
@@ -58,13 +58,7 @@ function main() {
     timedExecOrDie('gulp check-types');
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    if (!determineBuildTargets(buildTargets, FILENAME)) {
-      stopTimer(FILENAME, FILENAME, startTime);
-      process.exitCode = 1;
-      return;
-    }
-
+    const buildTargets = determineBuildTargets(FILENAME);
     reportAllExpectedTests(buildTargets);
     timedExecOrDie('gulp update-packages');
 

diff --git a/build-system/pr-check/dist-bundle-size.js b/build-system/pr-check/dist-bundle-size.js
@@ -54,13 +54,7 @@ function main() {
     uploadDistOutput(FILENAME);
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    if (!determineBuildTargets(buildTargets, FILENAME)) {
-      stopTimer(FILENAME, FILENAME, startTime);
-      process.exitCode = 1;
-      return;
-    }
-
+    const buildTargets = determineBuildTargets(FILENAME);
     if (
       buildTargets.has('RUNTIME') ||
       buildTargets.has('FLAG_CONFIG') ||

diff --git a/build-system/pr-check/e2e-tests.js b/build-system/pr-check/e2e-tests.js
@@ -46,9 +46,7 @@ async function main() {
     timedExecOrDie('gulp e2e --nobuild --headless');
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    determineBuildTargets(buildTargets, FILENAME);
-
+    const buildTargets = determineBuildTargets(FILENAME);
     if (
       buildTargets.has('RUNTIME') ||
       buildTargets.has('FLAG_CONFIG') ||

diff --git a/build-system/pr-check/local-tests.js b/build-system/pr-check/local-tests.js
@@ -48,9 +48,7 @@ function main() {
     timedExecOrDie('gulp codecov-upload');
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    determineBuildTargets(buildTargets, FILENAME);
-
+    const buildTargets = determineBuildTargets(FILENAME);
     if (
       !buildTargets.has('RUNTIME') &&
       !buildTargets.has('FLAG_CONFIG') &&

diff --git a/build-system/pr-check/remote-tests.js b/build-system/pr-check/remote-tests.js
@@ -53,9 +53,7 @@ async function main() {
     stopSauceConnect(FILENAME);
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    determineBuildTargets(buildTargets, FILENAME);
-
+    const buildTargets = determineBuildTargets(FILENAME);
     if (
       !buildTargets.has('RUNTIME') &&
       !buildTargets.has('FLAG_CONFIG') &&

diff --git a/build-system/pr-check/single-pass-tests.js b/build-system/pr-check/single-pass-tests.js
@@ -47,9 +47,7 @@ function main() {
     );
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    determineBuildTargets(buildTargets, FILENAME);
-
+    const buildTargets = determineBuildTargets(FILENAME);
     if (
       buildTargets.has('RUNTIME') ||
       buildTargets.has('FLAG_CONFIG') ||

diff --git a/build-system/pr-check/validator-tests.js b/build-system/pr-check/validator-tests.js
@@ -50,13 +50,7 @@ function main() {
     timedExecOrDie('gulp validator-webui');
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    if (!determineBuildTargets(buildTargets, FILENAME)) {
-      stopTimer(FILENAME, FILENAME, startTime);
-      process.exitCode = 1;
-      return;
-    }
-
+    const buildTargets = determineBuildTargets(FILENAME);
     if (
       !buildTargets.has('RUNTIME') &&
       !buildTargets.has('VALIDATOR') &&

diff --git a/build-system/pr-check/visual-diff-tests.js b/build-system/pr-check/visual-diff-tests.js
@@ -48,9 +48,7 @@ function main() {
     timedExecOrDie('gulp visual-diff --nobuild --master');
   } else {
     printChangeSummary(FILENAME);
-    const buildTargets = new Set();
-    determineBuildTargets(buildTargets, FILENAME);
-
+    const buildTargets = determineBuildTargets(FILENAME);
     process.env['PERCY_TOKEN'] = atob(process.env.PERCY_TOKEN_ENCODED);
     if (
       buildTargets.has('RUNTIME') ||

diff --git a/build-system/tasks/pr-check.js b/build-system/tasks/pr-check.js
@@ -55,13 +55,7 @@ async function prCheck(cb) {
   }
 
   printChangeSummary(FILENAME);
-  const buildTargets = new Set();
-  if (!determineBuildTargets(buildTargets, FILENAME)) {
-    stopTimer(FILENAME, FILENAME, startTime);
-    process.exitCode = 1;
-    return;
-  }
-
+  const buildTargets = determineBuildTargets(FILENAME);
   runCheck('gulp lint --local_changes');
   runCheck('gulp presubmit');