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

Move variable definitions from components to packages #621

Merged
merged 50 commits into from
Jul 29, 2022

Conversation

Racer159
Copy link
Contributor

@Racer159 Racer159 commented Jul 13, 2022

Description

Move variable declarations to the package level and enable additional features such as default and prompt.

Related Issue

Fixes #613 --> Moves variables to the package level instead of the component level
Fixes #489 --> Adds a cli flag to override variables in a package definition

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist before merging

  • Tests have been added/updated as necessary (add the needs-tests label)
  • Documentation has been updated as necessary (add the needs-docs label)
  • An ADR has been written as necessary (add the needs-adr label) [ 1 2 3 ]
  • (Optional) Changes have been linted locally with golangci-lint. (NOTE: We haven't turned on lint checks in the pipeline yet so linting may be hard if it shows a lot of lint errors in places that weren't touched by changes. Thus, linting is optional right now.)

@Racer159 Racer159 changed the title WIP update schema and examples to match vision Move variable definitions from components to packages Jul 13, 2022
@Racer159 Racer159 added needs-adr needs-tests PR Label - Tests required to merge and removed needs-adr labels Jul 13, 2022
@Racer159
Copy link
Contributor Author

We'll see how far this gets :D (will probably fail but it should kinda work)

@YrrepNoj YrrepNoj self-requested a review July 18, 2022 20:32
@Racer159 Racer159 removed the needs-tests PR Label - Tests required to merge label Jul 18, 2022
@Racer159
Copy link
Contributor Author

sorry for the new merge conflicts @Racer159 eyes

Should be fixed now.

@jeff-mccoy jeff-mccoy added enhancement ✨ New feature or request packager labels Jul 27, 2022
src/types/component.go Outdated Show resolved Hide resolved
src/types/package.go Outdated Show resolved Hide resolved
src/types/component.go Outdated Show resolved Hide resolved
adr/0006-package-variables.md Outdated Show resolved Hide resolved
adr/0006-package-variables.md Outdated Show resolved Hide resolved
adr/0006-package-variables.md Outdated Show resolved Hide resolved
src/internal/packager/compose.go Show resolved Hide resolved
jeff-mccoy
jeff-mccoy previously approved these changes Jul 28, 2022
Copy link
Contributor

@jeff-mccoy jeff-mccoy left a comment

Choose a reason for hiding this comment

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

🥳 lgtm. super excited to see this change. This one is bigger, so think we should have me & @YrrepNoj approve.

@jeff-mccoy
Copy link
Contributor

Everything lgtm, will wait for @YrrepNoj to approve since it's a bigger PR. Thanks @Racer159--outstanding work!

Copy link
Contributor

@YrrepNoj YrrepNoj left a comment

Choose a reason for hiding this comment

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

Super excited for these changes!

@jeff-mccoy jeff-mccoy merged commit c9426cc into master Jul 29, 2022
@jeff-mccoy jeff-mccoy deleted the 613-top-level-variables-for-packages branch July 29, 2022 20:20
@jeff-mccoy jeff-mccoy mentioned this pull request Jul 30, 2022
Noxsios pushed a commit that referenced this pull request Mar 8, 2023
* WIP update schema and examples to match vision

* Update schema to explicitly dissallow ###ZARF_VAR_ in default variable

* Set default variables on package create

* Create an initial pass at the --set flag

* Make package create test more robust

* Do not replace values when the default key is not present and --set is not used

* Cleanup package create test

* WIP start on deploy templating

* Create the weird one

* Mostly working now (needs more testing and stuff)

* Flesh out the variable test example

* Add more layers to more fully test variables

* Remove image from configmap

* Refactor to reduce complexity (i'm addicted pls help)

* Remove xtra comments

* Grammer fixes

* Add validation for variables on package create

* Add prompting to the deploy stage

* Update CLI docs with --set option

* Update Zarf schema

* Remove time from Zarf schema autogen

* Jeff breaking everything (though to be fair I reviewed his commit)

* Fixup feedback (rando example and debug message)

* Add --set to prepare find-images

* Remove inspect variables load (unnecessary)

* Address a TODO in a test

* Catch error from survey.AskOne()

* Update to new package variable/constant specification

* Fix and test for some potentially bad bugs

* Remove todo from SetActiveVariables

* Fix import path schema

* Small fixes to verbiage

* put a better comment in the package variable example

* Make Jeff's changes real

* Move variables logic into its own file

* Refactor SetActiveVariables to be better and include regex link for import path

* Fixing error around taking the default

* Update default to remove  and add adr

* Spelling mistakes and comments

* Fix default on confirm check

Co-authored-by: Jonathan Perry <[email protected]>
Co-authored-by: Megamind <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variable all the things! Add a CLI flag to override the custom Zarf variables
3 participants