amperity / lein-monolith

Leiningen plugin for working with monorepos.
Other
214 stars 18 forks source link

Dependency closure selection #16

Closed greglook closed 7 years ago

greglook commented 7 years ago

This PR adds a few new features to the plugin:

aengelberg commented 7 years ago
$ lein monolith each :parallel 6 install
Applying install to 88 subprojects...
java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Number
aengelberg commented 7 years ago

While in app root:

$ lein monolith each :downstream do
Applying do to 88 subprojects...

I'd argue this should throw an exception saying that you can't do :upstream or :downstream in the project root.

aengelberg commented 7 years ago

Looks like :start does not have the same name flexibility (namespace optional) as the rest of the options. Is that intentional?

aengelberg commented 7 years ago

Looks like :start does not play well with :in:

lein monolith each :in ingest-core,source-core :start amperity/ingest-core do
Applying do to 1 subprojects...

Applying to amperity/ingest-core
Completed amperity/ingest-core (1/1) in 0:00.355

SUCCESS: Applied do to 1 projects in 0:00.484
aengelberg commented 7 years ago

It would be cool if you could do a negative of an additive option. The specific use case I'm thinking of is that if a each :parallel task fails, I want to then run:

lein monolith each :parallel 6 :exclude-upstream project-that-failed
aengelberg commented 7 years ago

With all these sophisticated tools to describe a set of tasks, I think the "execute in dependency order" strict requirement might become a restriction for certain use cases that are now enabled. It would be nice to be able to turn that off for certain tasks that I don't really care the order they are executed. That would make :parallel faster in most subproject trees.

Example:

lein monolith each :parallel 6 :topological false ...

Use cases might be:

aengelberg commented 7 years ago

One feature I'm still hurting for is lein monolith link these,specific,projects. Seems somewhat related to the features you just added to each.

greglook commented 7 years ago

Thanks for the feedback! Ready for another look.

java.lang.ClassCastException: java.lang.String cannot be cast to java.lang.Number

Fixed, was missing an integer parse on the option.

I'd argue this should throw an exception saying that you can't do :upstream or :downstream in the project root.

Done.

Looks like :start does not have the same name flexibility

Good catch, added name resolution to it.

Looks like :start does not play well with :in

Going to punt on this, since I don't think these would overlap much.

It would be cool if you could do a negative of an additive option.

There's :skip, which lets you remove specific packages. I'm not sure I see the use-case for excluding upstream/downstream dependency trees.

I think the "execute in dependency order" strict requirement might become a restriction for certain use cases that are now enabled.

Hmm - for testing, you definitely do want to execute in dependency order still, since if an upstream package is broken it probably means your downstream packages will be impacted. Running multiple forever tasks is an interesting case, but again going to push that to a separate PR.

One feature I'm still hurting for is lein monolith link these,specific,projects

Yeah, that would be nice. I'd rather add that in a follow-up PR now that the groundwork for the option parsing and name resolution is in place.