Skip to content

feat: include coder_parameters from external modules #8195

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

Merged
merged 7 commits into from
Jun 26, 2023
Merged

Conversation

mtojek
Copy link
Member

@mtojek mtojek commented Jun 26, 2023

Fixes: #8090

This PR modifies the ConvertResources logic to include coder_parameters from external modules. It also refactors the routine to prevent nil resources.

@mtojek mtojek self-assigned this Jun 26, 2023
@mtojek mtojek changed the title feat: collect coder_parameters from external modules feat: include coder_parameters from external modules Jun 26, 2023
@mtojek mtojek marked this pull request as ready for review June 26, 2023 10:48
@mtojek mtojek requested review from johnstcn and mafredri June 26, 2023 10:48
Comment on lines 741 to 744
for _, o := range ordered {
if resource.Name == o.Name {
goto next
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't have anything in particular against the goto here, but continue outerLoop is likely to raise fewer eyebrows.

Copy link
Member Author

Choose a reason for hiding this comment

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

I hear you. I replaced goto with a simple boolean check.

Comment on lines 757 to 760
withExternal := make([]*tfjson.StateResource, 0, len(ordered)+len(external))
withExternal = append(withExternal, external...)
withExternal = append(withExternal, ordered...)
ordered = withExternal
Copy link
Member

Choose a reason for hiding this comment

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

This could be simplified:

Suggested change
withExternal := make([]*tfjson.StateResource, 0, len(ordered)+len(external))
withExternal = append(withExternal, external...)
withExternal = append(withExternal, ordered...)
ordered = withExternal
ordered = append(external, ordered...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

@mtojek mtojek enabled auto-merge (squash) June 26, 2023 13:36
@mtojek mtojek merged commit b4f14cc into main Jun 26, 2023
@mtojek mtojek deleted the 8090-module-support branch June 26, 2023 13:46
@github-actions github-actions bot locked and limited conversation to collaborators Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

coder_parameters of terraform module are not properly collected
3 participants