Read from raw project for inherit-raw and inherit-leaky-raw #75

robhanlon22 · 2020-09-26T22:48:03Z

Resolves #68.

src/lein_monolith/plugin.clj

src/lein_monolith/task/info.clj

src/lein_monolith/plugin.clj

src/lein_monolith/config.clj

robhanlon22 · 2020-09-27T00:23:41Z

It seems that with-all needs to be updated as well—needs to actually build and activate the inherited profiles before grabbing the paths.

EDIT: this is done

src/lein_monolith/plugin.clj

robhanlon22 · 2020-09-30T23:49:28Z

@greglook this is ready to go!

greglook

Overall this looks pretty good! A bunch of comments for you, covering a mix of style, idiom, and structuring, plus some questions about why you made certain changes.

.circleci/config.yml

dev/lein_monolith/test_utils.clj

example/apps/app-a/project.clj

greglook · 2020-10-01T19:44:13Z

src/lein_monolith/plugin.clj

+    profile-config))
+
+
+(def ^:private init-lock


Hmm, I really don't think that the plugin code should be doing any subproject initialization. That should only be needed when running tasks in the subprojects, which should only be in the each task.

src/lein_monolith/plugin.clj

src/leiningen/monolith.clj

test/example-tests.sh

test/leiningen/monolith_test.clj

robhanlon22 · 2020-10-01T21:23:13Z

Thank you for the thoughtful review! I'll address these comments ASAP.

This is to reduce spam in tests, and to allow printing of the entire output in exceptional cases.

robhanlon22 · 2020-10-02T01:16:21Z

@greglook ready for 👀 again! One thing that might be addressed on a follow-up is that I couldn't get values from composite profiles to correctly merge into the all profile. This is, however, the current behavior, so it should be addressed. Example: the app-a/bench directory isn't in the all profile, even though it should be.

robhanlon22 · 2020-10-02T02:14:19Z

Also, would you like me to squash this?

greglook · 2020-10-02T03:04:29Z

Thanks for the updates! I'll take another look.

Also, would you like me to squash this?

I'll squash when I merge.

robhanlon22 · 2020-10-02T06:08:23Z

@greglook ready for 👀 again! One thing that might be addressed on a follow-up is that I couldn't get values from composite profiles to correctly merge into the all profile. This is, however, the current behavior, so it should be addressed. Example: the app-a/bench directory isn't in the all profile, even though it should be.

I reverted some added behavior that attempted to activate all of the active profiles when building the child (re)source and test paths. It's back to its existing behavior, which is that paths under the profiles activated in the parent are not merged, e.g. if there are paths in the :dev profile in a child profile and :dev is active in the parent when running with-all, those paths are not merged.

robhanlon22 · 2020-10-02T17:59:39Z

@greglook here's the follow-up work, as discussed: robhanlon22#1 I opened it as a PR against this branch in my fork; I'll re-apply the changes against master and open a PR against upstream once this PR lands.

greglook

This looks good! One more minor comment and I'll merge this. Should be available later as part of a 1.6.0-SNAPSHOT release for testing.

src/lein_monolith/plugin.clj

test/leiningen/monolith_test.clj

robhanlon22 force-pushed the inherit-raw branch 3 times, most recently from a640bf0 to 3703a7d Compare September 26, 2020 23:06

robhanlon22 mentioned this pull request Sep 26, 2020

Inheriting :test-paths and other paths #68

Closed

robhanlon22 force-pushed the inherit-raw branch 2 times, most recently from 0c099d4 to be8c790 Compare September 26, 2020 23:08