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

Add --force-local to link command #1082

Merged
merged 3 commits into from
Oct 30, 2017
Merged

Add --force-local to link command #1082

merged 3 commits into from
Oct 30, 2017

Conversation

jiverson
Copy link
Contributor

@jiverson jiverson commented Oct 19, 2017

Add ignore sem versioning. Addresses #425, #908, and #928.

Description

Add in --force-local option for linking only.

Motivation and Context

Being able to switch branches during dev mode as well as using local packages. As well as using git subtree instead of mono repos.

How Has This Been Tested?

Yes added in integration tests.

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)

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.

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.

Looking good so far, thanks for your patience!

README.md Outdated
#### --ignore-semver

```sh
$ lerna link --ignore-semver=<boolean>
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this --force-local, please? Even though it does ignore semver, in practice, I don't want to implicitly condone ignoring semver. It's pure semantics, yes, but I think --force-local better describes what is actually being accomplished, rather that how it is implemented.

Also, for boolean CLI options, it's not necessary to pass a value:

$ lerna link --force-local

(omitting the flag entirely is the equivalent of false, the default)

@@ -521,6 +521,7 @@ export default class BootstrapCommand extends Command {
* @param {Function} callback
*/
symlinkPackages(callback) {
PackageUtilities.symlinkPackages(this.filteredPackages, this.packageGraph, this.logger, callback);
const { filteredPackages, packageGraph, logger } = this;
PackageUtilities.symlinkPackages(filteredPackages, packageGraph, logger, false, callback);
Copy link
Member

Choose a reason for hiding this comment

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

Can you label the false please? I dislike long lists of arguments with sudden literals in the middle.

+const forceLocal = false;
const { filteredPackages, packageGraph, logger } = this;
-PackageUtilities.symlinkPackages(filteredPackages, packageGraph, logger, false callback);
+PackageUtilities.symlinkPackages(filteredPackages, packageGraph, logger, forceLocal, callback);

README.md Outdated
$ lerna link --ignore-semver=<boolean>
```

This flag signifies whether or not the `link` command should ignore semantic version validation. Its default value is `false`.
Copy link
Member

Choose a reason for hiding this comment

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

Renaming to --force-local, a better description would be

When passed, this flag causes the `link` command to always symlink local dependencies regardless of matching version range.

@evocateur evocateur changed the title feat: add in ignore semver Add --force-local to link command Oct 27, 2017
@jiverson
Copy link
Contributor Author

Force local makes more sense appreciate the feed back.

@evocateur
Copy link
Member

Just waiting for the gummed-up CI to finish, then I'll merge. Thanks for responding quickly!

@evocateur evocateur merged commit 8122ead into lerna:master Oct 30, 2017
@jiverson jiverson deleted the feat/link/ignore-semver branch October 30, 2017 23:01
@Kikobeats
Copy link

one thing, because boostrap command implicitly make a link, have sense if boostrap supports --force-local flag as well?

@lock
Copy link

lock bot commented Dec 27, 2018

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 Dec 27, 2018
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.

None yet

3 participants