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

resolve require section from imports #2624

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dtomasi
Copy link
Contributor

@dtomasi dtomasi commented Apr 14, 2023

What issue type does this pull request address? (keep at least one, remove the others)
/kind bugfix

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)

This PR adds a special handling of the require section in loader.ResolveImports. Instead of overwriting command and plugin constraints, it appends them to the existing ones. This was not handled correctly before, as imported "libraries" overwrote the existing constraints.

It also changes the handling of constraints for devspace versios by adding the version constraints form imported files to a already existing constraint, separated by comma.
Example: >=6.0, >=6.1

Please provide a short message that should be published in the DevSpace release notes
Fixed an issue where DevSpace did not resolve command and plugin constraints from imports

What else do we need to know?

@netlify
Copy link

netlify bot commented Apr 14, 2023

Deploy Preview for devspace-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 0f6eb41
🔍 Latest deploy log https://app.netlify.com/sites/devspace-docs/deploys/643958c8fe9038000866e760

…gin constraints

Signed-off-by: Dominik Tomasi <dominik.tomasi@gmail.com>
@dtomasi dtomasi force-pushed the fix/append-require-constraints branch from a73d1c7 to 0f6eb41 Compare April 14, 2023 13:44
@89luca89
Copy link
Collaborator

89luca89 commented May 2, 2023

Hi @dtomasi

Thanks a lot for the PR!
Can I ask to add an e2e test for this? So we're sure we don't have regressions in the future

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.

None yet

2 participants