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

[f-nested-deps] Remove requirement to have root pack name in overrides #426

Merged
merged 6 commits into from
Oct 3, 2023

Conversation

angrycub
Copy link
Contributor

Continues on the work in #403. Removes the requirement to specify the name of the root pack in the override variable's path.

@angrycub angrycub self-assigned this Sep 22, 2023
@angrycub angrycub added type/enhancement theme/pack/dependency Relates to pack dependencies labels Sep 22, 2023
@angrycub angrycub force-pushed the f-nested-var-deps-cont branch from f7a8846 to b804429 Compare September 25, 2023 21:10
@angrycub angrycub changed the title [f-nomad-var-deps] Remove requirement to have root pack name in overrides [f-nested-deps] Remove requirement to have root pack name in overrides Sep 25, 2023
@angrycub angrycub force-pushed the f-nested-var-deps-cont branch from b804429 to ef3a0ca Compare September 26, 2023 16:13
Base automatically changed from f-nested-var-deps to main September 26, 2023 16:20
Copy link
Contributor

@pkazmierczak pkazmierczak left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -13,13 +13,15 @@ import (
"github.com/hashicorp/nomad-pack/sdk/pack/variables"
)

func DecodeVariableOverrides(files []*pack.File) DecodeResult {
// TODO: this is only used in a test - remove?
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved into the test package

var path pack.ID
var name variables.ID

// NOTE: This implementation assumes a single element variable value,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's fine for now but maybe make an issue so that we could revisit later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created issue #435 to capture this

@angrycub angrycub merged commit 53e6fff into main Oct 3, 2023
@angrycub angrycub deleted the f-nested-var-deps-cont branch October 3, 2023 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/pack/dependency Relates to pack dependencies type/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants