Lerna import by gigabo · Pull Request #173 · lerna/lerna

gigabo · 2016-06-02T19:55:45Z

The problem: I have a package in a stand-alone repository that I want to pull into a lerna repo.

The solution: lerna import.

Imports a package from a foreign repository with commit history.

gigabo · 2016-06-03T19:14:32Z

Pretty sweet progress bar experience with enough git history to import:

jamiebuilds · 2016-06-06T06:14:53Z

I really really like this. Thanks for doing this.

Can you use the name "external" instead of "foreign"?

jamiebuilds · 2016-06-06T06:16:30Z

src/commands/ImportCommand.js

+
+    this.commits.forEach(sha => {
+      progressBar.tick(sha);
+      const patch = this.foreignExecSync(`git format-patch -1 ${sha} --stdout`)


Can you add a comment explaining what is happening here?

gigabo · 2016-06-06T17:34:22Z

Can you use the name "external" instead of "foreign"?

Yes, that's a better name. Done.

hzoo · 2016-06-06T18:38:04Z

README.md

+$ lerna import <path-to-external-repository>
+```
+
+Import the package at `<path-to-external-repository>`, with commit history.


If we want to be more verbose here, you can mention that the package x is imported into packages/x and maybe an example. You can even use lerna-changelog as the example?

Added some more detail here. I worry that using lerna-changelog as an example might be confusing, since it has "lerna" in the name, so I continued to use placeholders.

I also lower-cased the section heading to match the changes from @kaicataldo over the weekend (which I've picked up via rebase).

Yeah you're right - maybe babel would of been a better example but all good

hzoo · 2016-06-07T14:15:54Z

README.md

+into `packages/<package-name>`.  Original commit authors, dates and messages
+are preserved.  Commits are applied to the current branch.
+
+This is useful for gathering pre-exising standalone packages into a Lerna


exising -> existing

Whoops! Thanks.

hzoo · 2016-06-07T14:47:31Z

src/commands/ImportCommand.js

+      if (!packageName) {
+        throw new Error(`No package name specified in "${packageJson}"`);
+      }
+      this.targetDir = "packages/"+packageName;


can do packages/${packageName} here

Sure, though I've tended not to when concatenating a single string constant with a single variable, since a template string is actually longer in that case. How about I add some space around the operator? That's the same length, but involves less punctuation. Seems to fit with context, too.

Yeah the style has been to usually have whitespace between operators (we should add space-infix-ops)

we should add space-infix-ops

#188

hzoo · 2016-06-07T22:08:22Z

👍

jamiebuilds · 2016-06-08T23:44:10Z

Can you add a test that --yes works?

gigabo · 2016-06-09T00:04:18Z

--yes was not supported. Added support and a test.

gigabo · 2016-06-09T00:07:42Z

Looks like the latest build may have had a spurious failure in one environment. I'm not able to retry.

jamiebuilds · 2016-06-09T01:21:04Z

--yes was not supported.

Weird, I thought PromptUtilities.confirm supported that all the time

hzoo · 2016-06-09T17:27:02Z

There's a merge conflict b/c of the last PR

gigabo · 2016-06-09T18:09:52Z

Rebased.

jamiebuilds · 2016-06-09T22:25:49Z

Oh fucking hell, merged another one of your PRs and it needs another rebase. 🃏

hzoo · 2016-06-09T22:27:07Z

blah I hope it's not because of the readme

gigabo · 2016-06-09T22:33:49Z

Rebased again.

blah I hope it's not because of the readme

Nope, lerna.js and index.js this time. 😛

hzoo · 2016-06-10T15:49:07Z

Would be nice to use lerna here and do a canary release - can try a publish manually

hzoo · 2016-06-11T17:10:23Z

src/commands/ImportCommand.js

+        throw new Error(`Input path "${inputPath}" is not a directory`);
+      }
+      const packageJson = path.join(externalRepoPath, "package.json");
+      const packageName = require(packageJson).name;


Do we want to make a specific error message here or no? (if the package path doesn't have a package.json)

Errored while running ImportCommand.initialize Error: Cannot find module '/module/package.json'

hzoo · 2016-06-11T17:13:18Z

Testing it out locally - do we want have an option to add a prefix or no? Like in babel everything is called babel-, and I assume when you import the packages are in like babel/generator and when you import you might want it to be called babel-generator (or for scoped modules)

maybe like lerna import ../generator --prefix generator or just the next be the new folder/package name like lerna import ../generator babel-generator ``lerna import ../generator --name babel-generator`

Not a blocker and can be a further improvement

hzoo · 2016-06-11T17:15:52Z

Another simple error we can add in is if you have already imported that repo in before (either by name or git log, package.json, etc)

gigabo · 2016-06-18T01:00:54Z

Rebased, added some tests, did these two things and got it running in node 0.10.

giuseppeg · 2016-06-18T08:32:52Z

src/commands/ImportCommand.js

+
+    this.targetDir = "packages/" + externalRepoBase;
+
+    let targetDirExists;


this block could be

try { fs.statSync(this.targetDir); } catch (e) { return callback(new Error(`Target directory already exists "${this.targetDir}"`)); }

right?

Yeah, I spent some time trying to come up with a simpler check here. What you posted above is nicer looking, but it is actually the inverse of what we want: It returns an error when the target directory does not exist.

Oh right my bad, read the code again now.
How about

try { if (fs.statSync(this.targetDir)) { return callback(new Error(`Target directory already exists "${this.targetDir}"`)); } } catch (e) { /* Pass */ }

This is a nit anyway :)

I like it! Always glad to be rid of a variable. 👹

hzoo · 2016-06-22T23:46:54Z

There's a docs fix to add, but other than that I'm good with landing this and we can improve as it's tested/used?

gigabo · 2016-06-23T00:19:28Z

Need to rebase again and back out the changes to support node 0.10.

Imports a package with git history from a foreign repository.

This is necessary for compatibility with lerna#188.

Also add tests for error messages

gigabo · 2016-06-23T00:35:12Z

Rebased and updated docs.

That last commit in lerna#173 updated _one_ reference to package name, but missed another.

That last commit in #173 updated _one_ reference to package name, but missed another.

* feature/pkg-main: Bootstrap: copy `main` property from package.json 2.0.0-beta.23 fix execSync use Add `onlyExplicitUpdates` flag so that only packages that have changed have version bumps rather than all packages that depend on the updated packages (lerna#241) Re-introduce node 0.10 support (lerna#248) 2.0.0-beta.22 Consistent naming in README `import` section (lerna#243) [skip ci] Lerna import (lerna#173) Revert "Use sync-exec for node 0.10" (lerna#242) Revert "Fix bootstrap install to use quotes around versions (lerna#235)" 2.0.0-beta.21 Fix bootstrap install to use quotes around versions (lerna#235) typo [skip ci] # Conflicts: # src/commands/BootstrapCommand.js

lock · 2018-12-28T01:17:49Z

This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs.

gigabo mentioned this pull request Jun 3, 2016

[Internal] Lerna should be a lerna repo #174

Closed

jamiebuilds reviewed Jun 6, 2016
View reviewed changes

hzoo reviewed Jun 6, 2016
View reviewed changes

gigabo force-pushed the lerna-import branch from 6941852 to a02b2c8 Compare June 6, 2016 19:23

hzoo reviewed Jun 7, 2016
View reviewed changes

hzoo added the enhancement label Jun 7, 2016

This was referenced Jun 7, 2016

Move pouchdb-collate into core #186

Closed

Move pouchdb-collate and pouchdb-collections into core apache/pouchdb#5295

Closed

hzoo reviewed Jun 7, 2016
View reviewed changes

gigabo mentioned this pull request Jun 7, 2016

Add "space-infix-ops" eslint rule #188

Merged

gigabo force-pushed the lerna-import branch from d30fa4e to fb12305 Compare June 9, 2016 18:09

gigabo force-pushed the lerna-import branch from fb12305 to c814618 Compare June 9, 2016 22:32

hzoo reviewed Jun 11, 2016
View reviewed changes

gigabo force-pushed the lerna-import branch 2 times, most recently from 7a0afd2 to 7f0ebd9 Compare June 18, 2016 00:10

giuseppeg reviewed Jun 18, 2016
View reviewed changes

gigabo added 13 commits June 22, 2016 17:26

Add lerna import command

12baf67

Imports a package with git history from a foreign repository.

Add tests for lerna import

a2925b1

Fix linter violations

865aa41

Import "external" repo instead of "foreign" repo

419e858

Add some commentary around mechanics of import

2c0dc70

Add some detail to the README entry for import

4d4915f

Fix a README typo

ea8fe24

Add some whitespace around a concatenation operator

7909627

Eliminate a "space-infix-ops" eslint rule violation

96114c3

This is necessary for compatibility with lerna#188.

Handle --yes flag in import command

e54eb19

Use directory name instead of package name during import

2c9fe67

Nicer error message for missing directory

cea0cdb

Also add tests for error messages

Don't import into pre-existing target directory

682321e

gigabo force-pushed the lerna-import branch from 34140e4 to 682321e Compare June 23, 2016 00:31

Don't claim to use package name during import in README

372de5a

hzoo merged commit 599aae4 into lerna:master Jun 23, 2016

gigabo added a commit to gigabo/lerna that referenced this pull request Jun 23, 2016

Consistent naming in README import section

8674110

That last commit in lerna#173 updated _one_ reference to package name, but missed another.

gigabo mentioned this pull request Jun 23, 2016

Consistent naming in README import section #243

Merged

hzoo pushed a commit that referenced this pull request Jun 23, 2016

Consistent naming in README import section (#243) [skip ci]

b7b4653

That last commit in #173 updated _one_ reference to package name, but missed another.

lock bot locked as resolved and limited conversation to collaborators Dec 28, 2018


		this.targetDir = "packages/" + externalRepoBase;

		let targetDirExists;

Conversation

gigabo commented Jun 2, 2016

Uh oh!

gigabo commented Jun 3, 2016

Uh oh!

jamiebuilds commented Jun 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gigabo commented Jun 6, 2016

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!

hzoo Jun 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

hzoo commented Jun 7, 2016

Uh oh!

jamiebuilds commented Jun 8, 2016

Uh oh!

gigabo commented Jun 9, 2016

Uh oh!

gigabo commented Jun 9, 2016

Uh oh!

jamiebuilds commented Jun 9, 2016

Uh oh!

hzoo commented Jun 9, 2016

Uh oh!

gigabo commented Jun 9, 2016

Uh oh!

jamiebuilds commented Jun 9, 2016

Uh oh!

hzoo commented Jun 9, 2016

Uh oh!

gigabo commented Jun 9, 2016

Uh oh!

hzoo commented Jun 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzoo commented Jun 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hzoo commented Jun 11, 2016

Uh oh!

gigabo commented Jun 18, 2016

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!

hzoo commented Jun 22, 2016

Uh oh!

gigabo commented Jun 23, 2016

Uh oh!

gigabo commented Jun 23, 2016

Uh oh!

lock bot commented Dec 28, 2018

Uh oh!

hzoo Jun 7, 2016 •

edited

Loading

hzoo commented Jun 11, 2016 •

edited

Loading