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

Unnecessarily Exported MaxInt in scheduler api types #82306

Closed
ahmad-diaa opened this issue Sep 4, 2019 · 7 comments · Fixed by #82732
Closed

Unnecessarily Exported MaxInt in scheduler api types #82306

ahmad-diaa opened this issue Sep 4, 2019 · 7 comments · Fixed by #82732
Assignees
Labels
sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling.

Comments

@ahmad-diaa
Copy link
Contributor

ahmad-diaa commented Sep 4, 2019

MaxInt const is exported in scheduler api

MaxInt = int(MaxUint >> 1)
although it is not needed to be exported. It is only used by plugins_test.go.

#81263 (comment) discussed that the common practice is to have API constants in defaults.go

/sig scheduling

@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 4, 2019
@ahmad-diaa
Copy link
Contributor Author

cc @alculquicondor @ahg-g

@ahg-g
Copy link
Member

ahg-g commented Sep 6, 2019

The problem is that it is part of a v1 API :(

@ahg-g
Copy link
Member

ahg-g commented Sep 6, 2019

Ok, it turns out they are not actually considered part of the API, and so we can remove them: #82283 (comment)

@ahmad-diaa
Copy link
Contributor Author

@ahg-g Should I open a PR to address removing those const values from types.go?

@ahg-g
Copy link
Member

ahg-g commented Sep 9, 2019

Yes, we can remove those in a followup PR to the one handling sized values.

@ahmad-diaa
Copy link
Contributor Author

Yes, this was the plan.
/assign

@Huang-Wei
Copy link
Member

@ahmad-diaa remember to remove MaxUint as well :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants