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.

Sign up for GitHub

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

Jump to bottom

data/aws/route53/variables: Drop unused extra_tags #685

Merged
merged 1 commit into from
Jan 10, 2019
Merged

data/aws/route53/variables: Drop unused extra_tags #685

merged 1 commit into from
Jan 10, 2019

Conversation

wking
Copy link
Member

@wking wking commented Nov 15, 2018 •
edited
Loading

The last consumer of these variables was removed by 4360905 (coreos/tectonic-installer#3231) back when they were tectonic_external_vpc_id and extra_tags. Now all the route53 module creates are
[aws_route53_record resources`][1], and they don't support tags.

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 15, 2018
@wking wking force-pushed the drop-route53-external-vpc-variable branch from 94e3f52 to d3e3042 Compare November 15, 2018 20:57
@wking wking changed the title data/aws/route53/variables: Drop unused external_vpc_id data/aws/route53/variables: Drop unused external_vpc_id and extra_tags Nov 15, 2018
@wking wking force-pushed the drop-route53-external-vpc-variable branch from d3e3042 to 4024edf Compare November 15, 2018 20:59
@abhinavdahiya
Copy link
Contributor

@wking the external_vpc_id is being removed here #654 i would rather review its deletion there.

@wking
Copy link
Member Author

wking commented Nov 15, 2018

the external_vpc_id is being removed here #654 i would rather review its deletion there.

I'm fine rebasing this on top of that one, but I think this removal here (where there are no consumers) is fairly separate from what #654 is doing (where there are also meaningful consumers). Also, this smaller change isn't a WIP, so we can land it now and put #654 on top (the rebase should be easy whichever PR ends up on top).

@abhinavdahiya
Copy link
Contributor

For ease of review and similar goal i setting hold until #654 merges

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 1, 2018
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2018
@wking
data/aws/route53/variables: Drop unused extra_tags
4f954f3 
The last consumer of these variables was removed by 4360905
(modulus,steps: enable existing vpc, 2018-05-16,
coreos/tectonic-installer#3231).  Now the route53 module only creates
aws_route53_record resources, and they don't support tags [1].

[1]: https://www.terraform.io/docs/providers/aws/r/route53_record.html
@wking wking force-pushed the drop-route53-external-vpc-variable branch from 4024edf to 4f954f3 Compare January 10, 2019 18:48
@wking wking changed the title data/aws/route53/variables: Drop unused external_vpc_id and extra_tags data/aws/route53/variables: Drop unused extra_tags Jan 10, 2019
@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 10, 2019
@wking
Copy link
Member Author

wking commented Jan 10, 2019

Rebased onto master with 4024edf -> 4f954f3, now that #654 landed. The only thing left for this PR is extra_tags.

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 10, 2019
@abhinavdahiya
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 10, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit c7efdb6 into openshift:master Jan 10, 2019
@wking wking deleted the drop-route53-external-vpc-variable branch January 10, 2019 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flaper87 flaper87 Awaiting requested review from flaper87

@hardys hardys Awaiting requested review from hardys

Assignees

@abhinavdahiya abhinavdahiya

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wking @abhinavdahiya @openshift-ci-robot @openshift-merge-robot