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.

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

fix(import): Ensure compatibility with root-level package globs #1875

Merged

Conversation

wavebeem
Copy link
Contributor

Fixes lerna import to work in the case where packages are housed at the root level of a lerna-managed repo and use a configuration like ["myapp-*"] for the packages configuration.

Description

I changed the getPackageDirectories() method inside the import command so that any path ending in * is considered a package root location instead of just the ones ending in /*.

Motivation and Context

This fixes #1872. The change makes it possible to use lerna import when your packages are located at the root directory of a lerna-managed repo.

How Has This Been Tested?

I added a new test that covers the exact case failing in the issue I reported. The test makes a new lerna repo with the myapp-* config, imports a subpackage, and then verifies the new history and file contents.

I had to add a new testing helper since having a static name for the test repo wasn't supported by the regular fixtures helper function.

I wrote the test before the implementation, verified it failed, wrote the implementation, and verified both manually and through the test code that everything is working correctly.

I ran npm test to test the code.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

I would imagine this is just a bug fix, not a breaking change, but I wouldn't know for sure.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I don't think the existing documentation needs to be updated. I expected this to work already based on it.

Note:

It's possibly outside the scope of this ticket, but the output of this error is hard to read when there are zero package directories since JS stringifies an empty array to empty string:

throw new ValidationError(
  "EDESTDIR",
  `--dest does not match with the package directories: ${this.getPackageDirectories()}`
);

Copy link
Member

@evocateur evocateur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@evocateur evocateur changed the title Fixes #1872 lerna import into root directory fix(import): Ensure compatibility with root-level package globs Jan 18, 2019
@evocateur evocateur merged commit 16ab98d into lerna:master Jan 18, 2019
@wavebeem wavebeem deleted the issue-1872-lerna-import-root-dir-bug branch January 19, 2019 00:48
@lock
Copy link

lock bot commented Mar 20, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Mar 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lerna import fails when package root is repo root
2 participants