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

[Feature] Add support for configurable package locations #365

Merged
merged 4 commits into from
Dec 14, 2016

Conversation

jamiebuilds
Copy link
Contributor

Just needs documentation. @nruhe you said you were interested? I would now, but I'm working on documentation for another project about to be open sourced

"version": "1.0.0",
"packages": [
{"glob": "packages/*/package.json"},
{"glob": "package-3/package.json"}

Choose a reason for hiding this comment

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

I would think that glob would be the name of a package here, but it looks like it globs paths to find a package.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that makes more sense as an API

Copy link
Contributor

@iamakulov iamakulov Oct 19, 2016

Choose a reason for hiding this comment

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

Why is a complex object even used here? An array of globs would be perfect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know...

Copy link
Contributor

Choose a reason for hiding this comment

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

I just submitted #393 which changes packages to an array of globs and does not require package.json to be specified, since I think that can be assumed. I also added a basic explanation to the README.

Copy link
Contributor

Choose a reason for hiding this comment

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

The tests in #393 pass locally and on Travis, so I'm assuming the Appveyor failures are on their end. Is there anything else I can do to improve the PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

@bclinkinbeard, you might find that #392 "helps" with one of the Appveyor failures (makes the test a little less platform-dependent).

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @darrylhodgins, I just pushed that up. :fingers-crossed:

Copy link
Contributor

Choose a reason for hiding this comment

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

@bclinkinbeard, I should have clarified: #392 doesn't solve the Appveyor permissions problem, just the path on that one test…

@steelbrain
Copy link
Contributor

No pressure but having this sooner would be nice :)

@darrylhodgins
Copy link
Contributor

I've added #392 yesterday to work around a unit test that was failing (but there seem to be permissions errors on AppVeyor).

@jamiebuilds jamiebuilds changed the title Add support for configurable package locations [Feature] Add support for configurable package locations Nov 16, 2016
@BenGale
Copy link

BenGale commented Nov 23, 2016

Hey All, any plans for when this will be merged in?

@andrewagain
Copy link

I really wanted this feature so I published this branch to npm here: @ahfarmer/lerna.

For those not familiar with scoped packages, the @ahfarmer/ is part of the package name. When this pull request is merged and an official release is pushed I will wait a week or two and then unpublish my package.

@gigabo
Copy link
Contributor

gigabo commented Dec 12, 2016

Okay, let's see if we can get this thing into shape and out the door.

The AppVeyor failures look real. I remember when we did this on Asini we had to make some tweaks to tests to get it working on Windows. Might just be a matter of adding some normalize(...) wrappers in the BootstrapCommand tests?

@rygine Does that sound right? Or was there also an actual code change we did?

@gigabo gigabo force-pushed the config-locations branch 7 times, most recently from c9a0e71 to 1c76b71 Compare December 13, 2016 23:01
gigabo and others added 4 commits December 14, 2016 11:48
This adds a `packages` configuration option to `lerna.json`:

```json
{
  "lerna": "x.x.x",
  "version": "0.2.3",
  "packages": [
    {"glob": "packages/{core,plugins}/*/package.json"},
    {"glob": "build-tools/package.json"},
    {"glob": "integration-tests/**/package.json"}
  ]
}
```

The default is `[{"glob": "packages/*/package.json"}]`, which is equivalent to
the historical layout of a Lerna repo.
… package.json (#393)

* Expect packages config to be an array of glob patterns to directories containing package.json

* Mention packages config in README

* Attempt to avoid test failures on Appveyor
@gigabo
Copy link
Contributor

gigabo commented Dec 14, 2016

Looks like the tests had just somehow gotten weird when this was reopened from #332.

I swapped out the first commit for a modified asini/asini#1, and added a commit at the end to deal with tests that changed while this PR was in-flight. Will merge this non-squash to preserve attribution for the contributions from @rygine and @bclinkinbeard.

This wound up with a nicer API than it started with. Thanks everyone!

@gigabo gigabo merged commit 1d14f9f into master Dec 14, 2016
@BenGale
Copy link

BenGale commented Dec 18, 2016

Just wanted to say I've switched back from our fork after this dropped and it's working great! Thanks for all the hard work guys, this project really makes a difference in the way we work.

@hzoo hzoo deleted the config-locations branch March 17, 2017 01:19
@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

10 participants