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

servicediscovery: refactoring to reduce code duplication #675

Merged
merged 4 commits into from
Jun 14, 2021

Conversation

muvaf
Copy link
Member

@muvaf muvaf commented May 25, 2021

Description of your changes

I merged #627 (review) to get the resource in, this PR makes little functional change but allows reuse of common hooks between all three HTTPNamespace, PrivateDNSNamespace and PublicDNSNamespace resources.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Manually.

@muvaf muvaf requested a review from hasheddan May 25, 2021 13:49
@muvaf
Copy link
Member Author

muvaf commented May 25, 2021

@blut you might want to take a look at this PR to see if I missed something. I believe only d038e7f is a functional change where I made it check the operation only if the external-name is not set, which happens only if the creation operation hasn't been triggered. The rest of the comments are mainly about reusing the observe and delete you implemented in all three of them withour duplicating it.

@muvaf
Copy link
Member Author

muvaf commented Jun 9, 2021

@krishchow can you take a look at this?

@krishchow
Copy link
Collaborator

Sounds good! Let me give this a review :)

Copy link
Collaborator

@krishchow krishchow left a comment

Choose a reason for hiding this comment

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

Aside from the failing check, this PR LGTM :)

…e small changes for resiliency of capturing external-name

Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
…space resources

Signed-off-by: Muvaffak Onus <me@muvaf.com>
…etOperation calls

Signed-off-by: Muvaffak Onus <me@muvaf.com>
@muvaf muvaf merged commit 20ced90 into crossplane-contrib:master Jun 14, 2021
@muvaf muvaf deleted the small-fixerolles branch June 14, 2021 18:40
tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024
tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants