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

implement addrs.ConfigResource #24362

Merged
merged 4 commits into from
Mar 13, 2020
Merged

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Mar 12, 2020

Core needs a way to address resources through unexpanded modules, as
they are present in the configuration. There are already some cases of
paring addrs.Module with addrs.Resource for this purpose, but it is
going to be helpful to have a single type to describe that pair, as
well as have the ability to use TargetContains.

@jbardin jbardin requested a review from a team March 12, 2020 18:50
@ghost ghost added the sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK label Mar 12, 2020
Remove unused variables, sync.Once, and init in BuiltinEvalContext.
Replace some shim calls.
GraphNodeAttachProvider doesn't need to be a NodeModuleInstance.
@jbardin jbardin force-pushed the jbardin/module-expansion-some-more branch from f50a0c1 to 6da0b1c Compare March 12, 2020 18:55
Core needs a way to address resources through unexpanded modules, as
they are present in the configuration. There are already some cases of
paring `addrs.Module` with `addrs.Resource` for this purpose, but it is
going to be helpful to have a single type to describe that pair, as
well as have the ability to use TargetContains.
@jbardin jbardin force-pushed the jbardin/module-expansion-some-more branch from 6da0b1c to d65bd64 Compare March 12, 2020 19:58
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

I like this 😄

@jbardin jbardin merged commit 50077ea into master Mar 13, 2020
@jbardin jbardin deleted the jbardin/module-expansion-some-more branch March 13, 2020 13:08
@ghost
Copy link

ghost commented Apr 13, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants