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

Update merged profile paths #83

Closed
wants to merge 2 commits into from

Conversation

robhanlon22
Copy link
Contributor

@robhanlon22 robhanlon22 commented Oct 2, 2020

Removes monolith project paths and dependencies from the output, and ensures that profiles active in the monolith are activated in subprojects before reading and absolutizing paths.

As discussed in #75.

* Don't include monolith project paths
* Ensure active profiles are activated in subprojects before reading and
  absolutizing paths
@robhanlon22
Copy link
Contributor Author

robhanlon22 commented Oct 2, 2020

This is failing on CI because it relies on a change to project-with-profiles that was introduced in Leiningen 2.9.4. I'll have to re-evaluate, unless lein-monolith wants to add a minimum Leiningen version requirement.

EDIT: I modified this to be less efficient, but it's now functional down to the version on CI.

absolutizes any paths in the subproject that may be relative."
[monolith [_project-name subproject]]
(-> subproject
(project/project-with-profiles)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@greglook
Copy link
Collaborator

greglook commented Oct 2, 2020

Hmm, 2.9.2 has been out for a while, but I'm not keen on adding a version requirement just for this specific edge case. I think this can be closed for now and maybe revisited if/when leiningen 3.0.0 becomes a thing.

@robhanlon22
Copy link
Contributor Author

Sounds good. I'm sure that when/if it becomes a thing, everything is going to break anyway 🤷‍♀️

@robhanlon22 robhanlon22 closed this Oct 2, 2020
@robhanlon22
Copy link
Contributor Author

To be clear, this does work on lein 2.9, but with that extensive profile reading, it might degrade the performance of with-all.

@robhanlon22 robhanlon22 reopened this Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants