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

helper funcs for converting rootcfgs to terraform roots #744

Merged
merged 12 commits into from
Apr 10, 2024

Conversation

smonero
Copy link
Contributor

@smonero smonero commented Apr 5, 2024

  • Note that I don't think we need things like TriggerInfo and plan mode so we don't set them.
  • Also I just use the string representations from the deploy package rather that copy/pasting them (EnvStep, Manifest, etc)
  • We could also use a WorkflowSignaler with TemporalClient nil to use the helper funcs like prependPlanEnvSteps but I figured it would be more straightforward to just copy them
  • We can also potentially move things to an internal folder similar to what is done with the pr and deploy flows, this would require changing some of the imports but not sure if that is even useful
  • Originally I wanted to reuse the structs from the deploy and pr flows (i.e. root, repo, step, etc), I decided to go with the deploy version rather than the pr version because deploy has the Apply job. However I am not 100% sure if that is actually necessary.

relies on #732 and #743 to be merged first, then we will be able to actually use these funcs.

@smonero smonero changed the title t helper funcs for converting rootcfgs to terraform roots Apr 5, 2024
Copy link
Contributor

@samrabelachew samrabelachew left a comment

Choose a reason for hiding this comment

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

Can you write tests for these changes?

server/neptune/workflows/adhoc.go Outdated Show resolved Hide resolved
@smonero smonero merged commit 723f4d6 into main Apr 10, 2024
3 checks passed
@smonero smonero deleted the terraformRootConv branch April 10, 2024 17:27
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