-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(route53): add fromPrivateHostedZoneAttributes function to PrivateHostedZone #35552
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
Hi aws-cdk maintainers, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tnb-git,
Thanks for your contribution :)
Overall looks good to me, just left two minor nit comments.
* Represents a Route 53 public hosted zone | ||
*/ | ||
export interface IPublicHostedZone extends IHostedZone {} | ||
export interface IPublicHostedZone extends IHostedZone { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a whitespace change. Could you please revert this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have addressed the whitespace change as requested.
* Represents a Route 53 private hosted zone | ||
*/ | ||
export interface IPrivateHostedZone extends IHostedZone {} | ||
export interface IPrivateHostedZone extends IHostedZone { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a whitespace change. Could you please revert this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have addressed the whitespace change as requested.
Pull request has been modified.
@abidhasan-aws Thank you for the review! I have addressed the comments, please take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks again for contributing.
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Pull request has been modified.
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
@Mergifyio requeue |
✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
1 similar comment
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
This pull request has been removed from the queue for the following reason: The pull request #35552 has been manually updated. If you want to requeue this pull request, you can post a |
Pull request has been modified.
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
Issue
Closes #23268 .
Reason for this change
PrivateHostedZone does not have fromPrivateHostedZoneAttributes function, Although PublicHostedZone has fromPublicHostedZoneAttributes.
this PR change allow us to reference the zoneName in CDK.
Description of changes
Added code for fromPrivateHostedZoneAttirbutes
Describe any new or updated permissions being added
No extra permissions needed
Description of how you validated changes
Added a unit test
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license