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

Introduce LocalOrigin + PathOrigin & schema.PathTarget #94

Merged
merged 6 commits into from
Nov 18, 2021

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Nov 8, 2021

This PR introduces a concept of a path-based origin, i.e. origin reference that is pointing to a different path, in Terraform that would be outside of a module.

In Terraform this can be represented by a module input name, pointing to the relevant variable, e.g.

module "example" {
  source = "./module"
  foo = "bar"
}

where foo would be the PathOrigin and point to this block within ./module/

variable "foo" {
  type = string
}

Relatedly all the existing origins become LocalOrigin with all the same existing structure.

There were a few high-level API changes in that both reference-lookup-related methods were moved under Decoder (from PathDecoder) as they can now both return results outside the context of a single path and therefore also need access to other paths via the PathReader.

It's not the smallest PR, but also it's not as big as some other recent ones 😅 and hopefully the commit log can be helpful in walking through it.

@radeksimko radeksimko requested a review from a team November 9, 2021 19:50
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

I was able to follow the changes in each commit and think everything makes sense so far. However, I cannot provide a "real" approval (yet) because everything is still new, and I can't oversee all implications.

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