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

Multiple mode dirs #23

Merged
merged 3 commits into from
May 15, 2024
Merged

Multiple mode dirs #23

merged 3 commits into from
May 15, 2024

Conversation

jonsmock
Copy link
Contributor

Updates mdc for the latest resolve-deps work, then adds support for multiple modes directories.

I previously incorrectly added a 'npm install' check into mdc, not seeing that mdc actually supported a path to not use resolve-deps at all. This seems useful, if we can sustain it (and it doesn't confuse users), so I first restore that functionality and move the check into run-tests.sh. We can test this continues to work using test4 by explicitly listing modes that would have been resolved.

To support multiple modes directories, we have the resolve-deps-less path return the same "node=/some/path" format that resolve-deps would. We check all modes directories and ensure that each mode in the mode spec is found and found only once.

As part of this, I added some comments to mdc to delineate creating base compose and files dir, then looping over modes to incorporate their contents, then finally user feedback ("MODES: ...") and setting env for COMPOSE_PROFILES/etc. The goal was to reduce the processing needed to show "MODES: ...", but it has a small advantage of consolidating the code to show all the env vars we set.

I considered reworking test4 to use multiple modes dirs, but it makes the test4 README fairly verbose.

@jonsmock jonsmock requested a review from kanaka May 13, 2024 20:06
Copy link
Member

@kanaka kanaka left a comment

Choose a reason for hiding this comment

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

See inline comment about dropping the fallback behavior and one other nit. Other than that, the rest looks great.

mdc Outdated Show resolved Hide resolved
mdc Outdated Show resolved Hide resolved
Removes deprecation warning.
Since 0.0.3 resolve-deps requires either --path or RESOLVE_DEPS_PATH.
resolve-deps supports multiple (comma-separated) directories for nodes
and can show the resolution in "node=/some/path" format if provided the
`--format=paths` flag option.  Use this to support multiple mdc modes
directories.

Reorder the echo/vecho lines a bit to reduce the processing required to
display the "MODES: .." line to the user.
@jonsmock jonsmock merged commit 245ef94 into master May 15, 2024
2 checks passed
@jonsmock jonsmock deleted the multiple-mode-dirs branch May 15, 2024 20:53
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