Add dependency unification script #293

ace-n · 2017-01-09T22:54:09Z

No description provided.

jmdobry · 2017-01-09T22:57:25Z

scripts/unify

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+const async = require(`async`);


In combination with #293 (comment)

jmdobry · 2017-01-09T22:58:19Z

scripts/unify

+const fs = require(`fs`);
+const path = require(`path`);
+
+const PROJECT_ROOT = path.dirname(__dirname);


Should be:

const PROJECT_ROOT = path.join(__dirname, '..');

jmdobry · 2017-01-09T22:59:32Z

scripts/unify

+// Dedupe package.json dependencies
+// WARNING: This will fail if two different versions of the same package are required.
+let pkgSet = {};
+let devPkgSet = {};


You should have to look at dev dependencies at all. They've all moved to the root package already.

jmdobry · 2017-01-09T23:00:08Z

scripts/unify

+
+// Get subdirectories with a `package.json` file
+const directories = fs.readdirSync(PROJECT_ROOT)
+                      .filter(dir => fs.existsSync(`${PROJECT_ROOT}/${dir}/package.json`));


Style nit: Change dir => to (dir) =>

Also, use path.join(PROJECT_ROOT, dir, 'package.json') instead of a template string.

jmdobry · 2017-01-09T23:01:13Z

scripts/unify

+let pkgSet = {};
+let devPkgSet = {};
+let pkgJson;
+for (const dir of directories) {


Prefer Array#forEach for arrays.

jmdobry · 2017-01-09T23:01:46Z

scripts/unify

+  pkgJson = JSON.parse(fs.readFileSync(`${PROJECT_ROOT}/${dir}/package.json`));
+
+  const deps = pkgJson[`dependencies`];
+  const devDeps = pkgJson[`devDependencies`];


Don't use bracket syntax, use pkgJson.dependencies

jmdobry · 2017-01-09T23:03:46Z

scripts/unify

+
+  for (p in deps) {
+    pkgSet[p] = deps[p];
+  }


Instead of these 3 lines just do:

Object.assign(pkgSet, deps);

jmdobry · 2017-01-09T23:04:05Z

scripts/unify

+
+// Update root-level package.json (by shelling to npm)
+const spawn = require('child_process').spawn;
+process.chdir(PROJECT_ROOT);


Remove line.

jmdobry · 2017-01-09T23:04:15Z

scripts/unify

+const spawn = require('child_process').spawn;
+process.chdir(PROJECT_ROOT);
+Object.keys(pkgSet).forEach(pkg => spawn(`yarn`, [`add`, `${pkg}@${pkgSet[pkg]}`]));
+Object.keys(devPkgSet).forEach(pkg => spawn(`yarn`, [`add`, `${pkg}@${devPkgSet[pkg]}`, `-D`]));


Remove line.

jmdobry · 2017-01-09T23:06:14Z

scripts/unify

+// Update root-level package.json (by shelling to npm)
+const spawn = require('child_process').spawn;
+process.chdir(PROJECT_ROOT);
+Object.keys(pkgSet).forEach(pkg => spawn(`yarn`, [`add`, `${pkg}@${pkgSet[pkg]}`]));


Instead of spawning 30+ processes, just spawn one:

spawn(`yarn`, [`add`].concat(Object.keys(pkgSet).map((pkg) => `${pkg}@${pkgSet[pkg]}`).join(' ')));

codecov-io · 2017-01-10T20:40:47Z

Current coverage is 91.78% (diff: 100%)

Merging #293 into master will decrease coverage by 0.28%

@@             master       #293   diff @@
==========================================
  Files            71         70     -1   
  Lines          2838       2702   -136   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           2613       2480   -133   
+ Misses          225        222     -3   
  Partials          0          0

Powered by Codecov. Last update f0f1e17...2bca91f

jmdobry · 2017-01-10T23:51:31Z

scripts/unify


 // Dedupe package.json dependencies
 // WARNING: This will fail if two different versions of the same package are required.
 let pkgSet = {};


This let can be a const

jmdobry · 2017-01-10T23:52:32Z

scripts/unify

+directories.forEach((dir) => {
+  pkgJson = JSON.parse(fs.readFileSync(path.join(PROJECT_ROOT, dir, `package.json`)));
+  Object.assign(pkgSet, pkgJson.dependencies);
+})


Missing semicolon. I don't think the lint script is seeing this file. We'll need to update the lint script to look at scripts/*

jmdobry · 2017-01-11T18:40:40Z

scripts/unify

+
+// Dedupe package.json dependencies
+// WARNING: This will fail if two different versions of the same package are required.
+let pkgSet = {};


This let can be a const

jmdobry · 2017-01-11T18:40:56Z

scripts/unify

+directories.forEach((dir) => {
+  pkgJson = JSON.parse(fs.readFileSync(path.join(PROJECT_ROOT, dir, `package.json`)));
+  Object.assign(pkgSet, pkgJson.dependencies);
+})


Missing semicolon; Looks like the linter isn't checking this file.

jmdobry · 2017-01-11T18:41:07Z

scripts/unify

+
+// Update root-level package.json (by shelling to npm)
+const spawn = require('child_process').spawn;
+spawn(`yarn`, [`add`].concat(Object.keys(pkgSet).map(pkg => `${pkg}@${pkgSet[pkg]}`)));


Missing newline at end of file

ace-n · 2017-01-11T22:22:17Z

~~Messed up a rebase; don't merge this until it's fixed.~~

Rebase has been fixed - feel free to review + merge.

googlebot · 2017-01-11T22:23:31Z

CLAs look good, thanks!

ace-n · 2017-01-11T23:46:33Z

@jmdobry it looks like the CircleCI failures are unrelated to this, so I'm merging this now. (If they are in fact related, let me know and I'll try to fix things.)

🤖 I have created a release \*beep\* \*boop\* --- ### [2.2.4](https://www.github.com/googleapis/nodejs-datalabeling/compare/v2.2.3...v2.2.4) (2021-06-30) ### Bug Fixes * **deps:** google-gax v2.17.0 with mTLS ([#291](https://www.github.com/googleapis/nodejs-datalabeling/issues/291)) ([ba455a8](https://www.github.com/googleapis/nodejs-datalabeling/commit/ba455a8d574f9bb87c9f7531ba56e199ad0dc8be)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release \*beep\* \*boop\* --- ### [3.1.1](https://www.github.com/googleapis/nodejs-containeranalysis/compare/v3.1.0...v3.1.1) (2021-04-27) ### Bug Fixes * specify valid go_package option ([#293](https://www.github.com/googleapis/nodejs-containeranalysis/issues/293)) ([84ba201](https://www.github.com/googleapis/nodejs-containeranalysis/commit/84ba201c2d2eb81faf36461cf1ff3bfccf33f5ed)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

There are some slight breaking changes in the newest UUID.

* docs: add base samples for automl ga * update package.json * lint fix * License header update * Fix cleanup on GCS prefix * Lint Co-authored-by: Benjamin E. Coe <bencoe@google.com>

* [CHANGE ME] Re-generated to pick up changes in the API or client library generator. * add protos to eslintignore * chore: hunted down missing videos * chore: fix video path * test: other resources are missing, let's use the cat for now * chore: updating URLs

* docs: add base samples for automl ga * update package.json * lint fix * License header update * Fix cleanup on GCS prefix * Lint Co-authored-by: Benjamin E. Coe <bencoe@google.com>

* [CHANGE ME] Re-generated to pick up changes in the API or client library generator. * add protos to eslintignore * chore: hunted down missing videos * chore: fix video path * test: other resources are missing, let's use the cat for now * chore: updating URLs

* docs: add base samples for automl ga * update package.json * lint fix * License header update * Fix cleanup on GCS prefix * Lint Co-authored-by: Benjamin E. Coe <bencoe@google.com>

ace-n requested a review from jmdobry January 9, 2017 22:54

jmdobry requested changes Jan 9, 2017

View reviewed changes

jmdobry assigned ace-n Jan 9, 2017

ace-n force-pushed the add-unify branch 2 times, most recently from 7b02ead to 2bca91f Compare January 10, 2017 20:33

jmdobry requested changes Jan 10, 2017

View reviewed changes

jmdobry requested changes Jan 11, 2017

View reviewed changes

Ace Nassri added 4 commits January 11, 2017 14:23

Add dependency unification script

a715cc0

Address comments

3acb0fd

Add use strict to unify script

b2ab69c

Address comments + fix lint

81e6bc4

ace-n force-pushed the add-unify branch from fb6bd5b to 81e6bc4 Compare January 11, 2017 22:23

jmdobry approved these changes Jan 11, 2017

View reviewed changes

ace-n merged commit 4e82315 into master Jan 11, 2017

ace-n deleted the add-unify branch January 11, 2017 23:47

ivanmkc mentioned this pull request Nov 3, 2022

Migrate code from googleapis/nodejs-datalabeling #2797

Merged

NimJay pushed a commit that referenced this pull request Nov 11, 2022

chore: update UUID to latest (#293)

5dbf7ff

There are some slight breaking changes in the newest UUID.

NimJay mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-kms #2839

Merged

ace-n pushed a commit that referenced this pull request Nov 11, 2022

chore: update UUID to latest (#293)

089edee

There are some slight breaking changes in the newest UUID.

ace-n mentioned this pull request Nov 11, 2022

migrate code from googleapis/nodejs-automl #2841

Merged

ace-n mentioned this pull request Nov 16, 2022

migrate code from googleapis/nodejs-video-intelligence #2855

Merged

Shabirmean pushed a commit that referenced this pull request Feb 16, 2023

feat: auto-generate READMEs, add .repo-metdata.json (#293)

ffa3135

Shabirmean mentioned this pull request Feb 16, 2023

samples: migrate code from googleapis/nodejs-dlp #3035

Merged

Add dependency unification script #293

Add dependency unification script #293

Uh oh!

Conversation

ace-n commented Jan 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jan 10, 2017

Current coverage is 91.78% (diff: 100%)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ace-n commented Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Jan 11, 2017

Uh oh!

ace-n commented Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ace-n commented Jan 11, 2017 •

edited

Loading

ace-n commented Jan 11, 2017 •

edited

Loading