worker: Rename worker_interface to worker #70
Conversation
|
@gaocegege The worker is an instance of evaluating a trial. |
|
I think worker suits it, because it represents the worker to run these trials 🤔 WDYT @ddysher |
|
conceptual-wise, i agree with @YujiOshima that |
cmd/manager/main.go
Outdated
| @@ -41,7 +41,7 @@ type studyCh struct { | |||
| addMetricsCh chan string | |||
| } | |||
| type server struct { | |||
| wIF worker_interface.WorkerInterface | |||
| wIF worker.WorkerInterface | |||
There was a problem hiding this comment.
Can we change WorkerInterface to Interface, per
https://github.com/golang/go/wiki/CodeReviewComments#package-names
example:
https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/cloud.go
|
Addressed |
|
LGTM @YujiOshima do you want to take another look? |
|
@ddysher Thanks! LGTM the name worker.Interface ! |
|
/lgtm |
|
@gaocegege: you cannot LGTM your own PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaocegege, YujiOshima 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 |
|
I am not sure why tide does not merge it automatically. Same as kubeflow/training-operator#557 |
|
It is too hack.. We can not merge a PR without lgtm although we have already added approve. |
worker_interface is a little misunderstanding, since it is not just a package for interface.
/assign @YujiOshima
Signed-off-by: Ce Gao gaoce@caicloud.io