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.

Sign up for GitHub

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

Jump to bottom

🚸 : mandatory variables #129

Merged
merged 15 commits into from
Aug 30, 2019
Merged

🚸 : mandatory variables #129

merged 15 commits into from
Aug 30, 2019

Conversation

juwit
Copy link
Member

@juwit juwit commented Aug 27, 2019

No description provided.

@juwit juwit added this to the 1.2.0 milestone Aug 28, 2019
@juwit juwit added ✨ enhancement New feature or request ✨ feature:modules module list & module edition ✨ feature:stacks stack list & stack edition 👤 user:admin admin 👤 user:end-user end-user labels Aug 28, 2019
@juwit juwit force-pushed the 72-mandatory-variables branch 7 times, most recently from 3d4c3f0 to 2dd6033 Compare August 30, 2019 07:37
@juwit juwit changed the title 🚧 : mandatory variables 🚸 : mandatory variables Aug 30, 2019
@juwit juwit marked this pull request as ready for review August 30, 2019 07:59
@juwit juwit requested a review from cdubuisson August 30, 2019 07:59
@juwit juwit added the 👌 ready for review The pull request is ready for review label Aug 30, 2019
cdubuisson
cdubuisson previously approved these changes Aug 30, 2019
View reviewed changes
src/main/java/io/codeka/gaia/stacks/service/MandatoryStackVariablesValidator.java Outdated
// getting the module
var module = this.moduleRepository.findById(stack.getModuleId()).orElseThrow();

for(TerraformVariable variable : module.getVariables()){
Copy link
Member

Choose a reason for hiding this comment

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

why not a java stream with operators filter and anyMatch ?

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea, using allMatch :

return module.getVariables().stream()
                .filter(TerraformVariable::isMandatory)
                .allMatch(variable -> {
                    var variableValue = stack.getVariableValues().get(variable.getName());
                    return variableValue != null && !variableValue.isBlank();
                });

src/main/java/io/codeka/gaia/stacks/service/RegexStackVariablesValidator.java Outdated
// getting the module
var module = this.moduleRepository.findById(stack.getModuleId()).orElseThrow();

for(TerraformVariable variable : module.getVariables()){
Copy link
Member

Choose a reason for hiding this comment

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

same here I guess

@cdubuisson cdubuisson merged commit 408efe8 into master Aug 30, 2019
@cdubuisson cdubuisson deleted the 72-mandatory-variables branch August 30, 2019 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdubuisson cdubuisson cdubuisson approved these changes

Assignees
No one assigned
Labels
✨ enhancement New feature or request ✨ feature:modules module list & module edition ✨ feature:stacks stack list & stack edition 👌 ready for review The pull request is ready for review 👤 user:admin admin 👤 user:end-user end-user
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@juwit @cdubuisson