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

refactoring: allow cross-package move statements #31556

Merged
merged 1 commit into from Aug 16, 2022

Conversation

kmoe
Copy link
Member

@kmoe kmoe commented Aug 2, 2022

Remove validation errors on cross-package move statements.

@kmoe kmoe force-pushed the kmoe/moved-external-modules branch from 5b44c3c to a1fc4b8 Compare August 2, 2022 15:17
@kmoe kmoe requested a review from a team August 12, 2022 09:43
@kmoe kmoe marked this pull request as ready for review August 12, 2022 09:43
Copy link
Member

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

This looks good to me!


I wonder if we should replace the now-obsolete paragraph in the documentation with some guidance about how to safely implement a cross-package move. 🤔

For example, I think we'd planned to recommend setting at least a minimum version pin on the module call to make sure the child module is at least new enough to either contain the resource(s) being moved to or to not contain the resource(s) being moved from. It follows from that advice that one must publish the new version of the package containing the child module that has the expected additions/removals before adding the upstream moved blocks.

I think there were some other items of guidance we discussed during design work too, but I'm not recalling them off the top of my head right now.

I'm admittedly not really sure where best to introduce this information and how to structure it. I don't think just inserting it directly in place of the removed paragraph would make sense because we'll probably have more than just the one short paragraph to include, but perhaps we could add a new section later in the page and link to it as an additional sentence on the end of the paragraph just before, which mentions that this example is for moves between modules in a single package.

No harm in looking into this in a separate PR though, since this PR won't leave the documentation incorrect, and the guidance we'd add does follow logically from other Terraform behaviors and thus experienced Terraform users can probably figure it out based on what they already know.

@kmoe kmoe merged commit 56a1e0d into main Aug 16, 2022
@kmoe kmoe deleted the kmoe/moved-external-modules branch August 16, 2022 14:53
@github-actions
Copy link

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2022
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.

None yet

2 participants