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

Support recursive targeting on modules #9236

Closed

Conversation

uhef
Copy link

@uhef uhef commented Oct 5, 2016

The purpose of this PR is to fix #5190 and other related issues. A new resource type "module" is introduced and if the targeted resource is a module then instead of targeting only the resources, all resources in the resource hierarchy under the targeted module are targeted.

@CarlInglisBJSS
Copy link

We pulled the master and merged the branch, then did some basic, unstructured and informal, testing with a multi-module layered TF configuration. It worked as expected.

@fatmcgav
Copy link
Contributor

@uhef What's the latest on this PR?

If you're happy that it's ready, removing the 'WIP' tag should trigger a review...

@uhef uhef changed the title [WIP] Support recursive targeting on modules Support recursive targeting on modules Nov 28, 2016
@uhef
Copy link
Author

uhef commented Nov 28, 2016

Removed the WIP tag.

@fatmcgav
Copy link
Contributor

@stack72 Any chance of a review on this one? :)

@sachincab
Copy link

+1

@uhef
Copy link
Author

uhef commented Jan 31, 2017

How does the review process unfold? Anything I can help here with anymore or is it up to the maintainers?

@cswilliams
Copy link

Hope this can get merged in soon! I really need this fix so I've been compiling my own version using this patch on v0.8.6 for the last week or so. I haven't run into any issues.

@Jaff
Copy link

Jaff commented Feb 28, 2017

+1 from our team

@dominicbarnes
Copy link

I'd really like to see this merged as well, it's causing me huge headaches atm.

@apparentlymart
Copy link
Contributor

Hi all! Thanks for the work here and sorry we let this sit here for so long.

This is a breaking change to the behavior of -target (it makes it match potentially more resources than it did before) so we'll need to wait until a major release to merge this as is our usual policy for breaking changes. Hopefully we can get this reviewed and merged for 0.10.0.

@uhef
Copy link
Author

uhef commented Apr 14, 2017

Great news!

Let me know if there's anything I can help you with!

@dcosson
Copy link
Contributor

dcosson commented May 4, 2017

@apparentlymart any timeline for the 0.10 release? I'm eagerly awaiting this fix, without it -target is just about useless when working with complicated modules and I have to plan my entire environment on every change.

@apparentlymart
Copy link
Contributor

apparentlymart commented May 4, 2017

Work on 0.10 is in progress but it's too early for us to have a firm release date. I can empathize with the frustration caused here! We have some other big things we need to get in before we can cut 0.10, though.

@pikeas
Copy link
Contributor

pikeas commented May 12, 2017

@apparentlymart Can you confirm whether this will definitely make it into 0.10?

@apparentlymart
Copy link
Contributor

@pikeas unfortunately I cannot see the future, so I can't say definitely, but we will have a look at this again once the final 0.9 release has been cut from master and we start to use master for 0.10 integration (not yet scheduled, since we're still doing 0.9 maintenance alongside 0.10 development) and see what work is required to get it merged.

It looks pretty solid from an initial pass of review, so I'm optimistic.

@jemmyw
Copy link

jemmyw commented Jun 1, 2017

@uhef any chance you could fix the merge conflicts? I've been using your patch on top of the 0.8 series, but it does not apply to 0.9

@josephholsten
Copy link
Contributor

@jemmyw care to take a look at #15293? it's just a rebase of this up to master

@apparentlymart
Copy link
Contributor

This was fixed in v0.10.0-beta1, by #15314. Sorry I forgot to close this out until now!

@ghost
Copy link

ghost commented Apr 8, 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 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

-target does not source child modules