chenyulin0719 commented on code in PR #1065:
URL: https://github.com/apache/yunikorn-k8shim/pull/1065#discussion_r3790867715
##########
pkg/plugin/predicates/predicate_manager.go:
##########
@@ -46,8 +46,8 @@ import (
type PredicateManager interface {
EventsToRegister(queueingHintFn fwk.QueueingHintFn)
[]fwk.ClusterEventWithHint
- Predicates(pod *v1.Pod, node *framework.NodeInfo, allocate bool)
(plugin string, error error)
- PreemptionPredicates(pod *v1.Pod, node *framework.NodeInfo, victims
[]*v1.Pod, startIndex int) (index int)
+ Predicates(pod *v1.Pod, node *framework.NodeInfo, allocate bool)
(string, error)
+ PreemptionPredicates(pod *v1.Pod, node *framework.NodeInfo, victims
[]*v1.Pod, startIndex int) int
Review Comment:
fter removing the named returns, there's no doc comment anywhere on this
interface. So anyone reading it has to infer:
- `Predicates()` return a string, what is it?
- `PreemptionPredicates` return a int, what is it?
I'd suggest keeping the semantics via a short doc comment instead of just
dropping the name.
##########
pkg/plugin/predicates/predicate_manager.go:
##########
@@ -46,8 +46,8 @@ import (
type PredicateManager interface {
EventsToRegister(queueingHintFn fwk.QueueingHintFn)
[]fwk.ClusterEventWithHint
- Predicates(pod *v1.Pod, node *framework.NodeInfo, allocate bool)
(plugin string, error error)
- PreemptionPredicates(pod *v1.Pod, node *framework.NodeInfo, victims
[]*v1.Pod, startIndex int) (index int)
+ Predicates(pod *v1.Pod, node *framework.NodeInfo, allocate bool)
(string, error)
+ PreemptionPredicates(pod *v1.Pod, node *framework.NodeInfo, victims
[]*v1.Pod, startIndex int) int
Review Comment:
After removing the named returns, there's no doc comment anywhere on this
interface. So anyone reading it has to infer:
- `Predicates()` return a string, what is it?
- `PreemptionPredicates` return a int, what is it?
I'd suggest keeping the semantics via a short doc comment instead of just
dropping the name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
