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

[issue-687] Refactor/operator register api #688

Merged
merged 6 commits into from
Nov 23, 2021
Merged

[issue-687] Refactor/operator register api #688

merged 6 commits into from
Nov 23, 2021

Conversation

heesuk-ahn
Copy link
Contributor

@heesuk-ahn heesuk-ahn commented Nov 17, 2021 •
edited
Loading

Proposal

  • Change deprecated kubernetesClient.getVersion to KubernetesClient.getKubernetesVersion
  • Pass a non-null value as a parameter by creating defaultConfiguration in register(Reconciler reconciler)
  • Remove unnecessary null defense code

by #687

@heesuk-ahn heesuk-ahn changed the base branch from master to v2 November 17, 2021 14:15
@heesuk-ahn heesuk-ahn mentioned this pull request Nov 17, 2021
Closed
csviri
csviri reviewed Nov 18, 2021
View reviewed changes
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java Outdated
controllers.add(controller);

final var watchedNS =
configuration.watchAllNamespaces() ? "[all namespaces]" : configuration.getEffectiveNamespaces();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line does not seems to be formatted if you just build the project with maven it should fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the reply :)
I rebuild maven and made a commit by modifying the code formats.

@csviri
Copy link
Collaborator

csviri commented Nov 18, 2021

Just made a minor not, otherwise LGTM!! Thank you!

@csviri csviri requested a review from metacosm November 18, 2021 08:27
@heesuk-ahn
Copy link
Contributor Author

As #678 was merged into the base branch v2, there was a conflict with some codes of the PR. I'll fix this part and rebase it. :)

@heesuk-ahn heesuk-ahn closed this Nov 22, 2021
@heesuk-ahn
Copy link
Contributor Author

As #678 was merged into the base branch v2, there was a conflict with some codes of the PR. I'll fix this part and rebase it. :)

Conflict has been corrected and rebase has been completed with v2. :)

@heesuk-ahn heesuk-ahn reopened this Nov 22, 2021
@csviri csviri merged commit 319ebee into operator-framework:v2 Nov 23, 2021
@csviri
Copy link
Collaborator

csviri commented Nov 23, 2021

thank you @heesuk-ahn !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@metacosm metacosm metacosm approved these changes

@csviri csviri csviri approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@heesuk-ahn @csviri @metacosm