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

WIP: [not merge] - just todo's and questions #1

Closed
wants to merge 1 commit into from
Closed

WIP: [not merge] - just todo's and questions #1

wants to merge 1 commit into from

Conversation

camilamacedo86
Copy link
Contributor

No description provided.

@camilamacedo86
Copy link
Contributor Author

/hold

@camilamacedo86 camilamacedo86 changed the title [not merge] - just todo's and questions WIP: [not merge] - just todo's and questions Jun 17, 2020
camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
main.go
Comment on lines 103 to 105
// Deprecated: --max-workers flag does not align well with the name of the option it configures on the controller
// (MaxConcurrentReconciles). Flag `--max-concurrent-reconciles` should be used instead.
if pflag.Lookup("max-workers").Changed {
Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Jun 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Should we not to do this deprecations now in SDK (ASAP)?

Copy link
Member

Choose a reason for hiding this comment

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

I need to think about this one more. I definitely see what you're getting at (i.e. let's get --max-workers removed before 1.0), but it's not super important for us to make this name change. We could just leave --max-workers and revert the switch to --max-concurrent-reconciles.

camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
camilamacedo86
camilamacedo86 commented Jun 17, 2020
View reviewed changes
joelanford
joelanford reviewed Jun 18, 2020
View reviewed changes
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

I think I answered everything :)

@camilamacedo86 camilamacedo86 mentioned this pull request Jun 21, 2020
Closed
camilamacedo86
camilamacedo86 commented Jun 21, 2020
View reviewed changes
@camilamacedo86 camilamacedo86 mentioned this pull request Jun 21, 2020
Closed
@camilamacedo86
Copy link
Contributor Author

Closing it since all was addressed.

SimonBaeumer added a commit to SimonBaeumer/helm-operator that referenced this pull request Sep 25, 2021
@SimonBaeumer
Add annotation package docs (operator-framework#1)
963a1cf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelanford joelanford joelanford left review comments

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

Successfully merging this pull request may close these issues.
2 participants
@camilamacedo86 @joelanford