-
Notifications
You must be signed in to change notification settings - Fork 39.1k
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
[scheduler cleanup phase 1]: Move FakeCache to pkg/scheduler/internal… #69318
[scheduler cleanup phase 1]: Move FakeCache to pkg/scheduler/internal… #69318
Conversation
ba1f177
to
75f9373
Compare
/kind cleanup |
/assign @k82cn |
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 think you need to update the users to use the new import path.
75f9373
to
e319f57
Compare
e319f57
to
558e196
Compare
// FakeCache is used for testing | ||
type FakeCache struct { | ||
// Cache is used for testing | ||
type Cache struct { |
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.
Rewrite FakeCache to Cache for
Errors from golint:
pkg/scheduler/internal/cache/fake/fake_cache.go:26:6: type name will be used as fake.FakeCache by other packages, and that stutters; consider calling this Cache
/retest |
/ping @misterikkit @k82cn @bsalamat |
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and | |||
limitations under the License. | |||
*/ | |||
|
|||
package testing | |||
package fake |
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.
Why not putting the file in scheduler/internal/cache
instead of scheduler/internal/cache/fake
?
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.
Since the fake is meant for unit testing, we want a way to avoid importing it into production binaries. One way would be to call the file fake_test.go
or we could just put it in a sub-package. Incidentally, this copies the convention from k8s.io/client-go/kubernetes/fake
.
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
/ping @k82cn Please take a look :) |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bsalamat, wgliang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
…/cache/fake
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #68959
Special notes for your reviewer:
@misterikkit
Release note: