Lint:fix lints #841

Thor-wl · 2020-05-29T08:55:57Z

Try to address issue #791

add some golangci-lint rules
fix some items found by golangci-lint

volcano-sh-bot · 2020-05-29T08:56:00Z

Welcome @Thor-wl!

It looks like this is your first PR to volcano-sh/volcano 馃帀.

Thank you, and welcome to Volcano. 😃

TravisBuddy · 2020-05-29T08:59:15Z

Travis tests have failed

Hey @Thor-wl,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: a94101b0-a18a-11ea-b485-534d9f3e4954

TravisBuddy · 2020-05-29T09:35:48Z

Travis tests have failed

Hey @Thor-wl,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: c40b4690-a18f-11ea-b485-534d9f3e4954

k82cn · 2020-05-29T10:42:53Z

pkg/client/clientset/versioned/typed/scheduling/v1beta1/scheduling_client.go

-	if err := setConfigDefaults(&config); err != nil {
-		return nil, err
-	}
+	setConfigDefaults(&config)


those are generated code, so please do not modify them.

TravisBuddy · 2020-05-30T01:50:13Z

Travis tests have failed

Hey @Thor-wl,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: e51900b0-a217-11ea-a8e9-e7e13745aead

pkg/scheduler/actions/preempt/preempt.go

k82cn · 2020-05-30T02:39:36Z

pkg/scheduler/actions/reclaim/reclaim.go

@@ -50,7 +50,7 @@ func (ra *Action) Execute(ssn *framework.Session) {
 	klog.V(3).Infof("There are <%d> Jobs and <%d> Queues in total for scheduling.",
 		len(ssn.Jobs), len(ssn.Queues))

-	var underRequest []*api.JobInfo
+	// var underRequest []*api.JobInfo


why remove this?

It is useless

useless variable has been already deleted

k82cn · 2020-05-30T02:39:59Z

pkg/scheduler/cache/event_handlers.go

 	if sc.Nodes[node.Name] != nil {
 		sc.Nodes[node.Name].SetNode(node)
 	} else {
 		sc.Nodes[node.Name] = schedulingapi.NewNodeInfo(node)
 	}
-
-	return nil


let's keep this.

k82cn · 2020-05-30T02:40:25Z

pkg/scheduler/cache/event_handlers.go

-		klog.Errorf("Failed to add node %s into cache: %v", node.Name, err)
-		return
-	}
+	sc.addNode(node)


let's keep oldNode, if not used, try _.

k82cn · 2020-05-30T02:41:49Z

please do not change the signature of function in this PR.

TravisBuddy · 2020-05-30T03:32:36Z

Travis tests have failed

Hey @Thor-wl,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: 3248f300-a226-11ea-a8e9-e7e13745aead

Signed-off-by: Thor <[email protected]>

hzxuzhonghu · 2020-05-30T07:28:09Z

.golangci.yml

+linters-settings:
+  lll:
+    line-length: 256
+  funlen:


I think we can disable this check. In the following pr, we need to first enable that with high priority.

As to the priority, If we have no enough time to analyze I suggest looking at https://github.com/istio/istio/blob/494aca43715d2f970bdb76ce1dfcc5e1095f3070/common/config/.golangci.yml#L34-L53

hzxuzhonghu · 2020-05-30T07:31:05Z

pkg/scheduler/actions/reclaim/reclaim.go

 		}

 		if len(job.TaskStatusIndex[api.Pending]) != 0 {
 			if _, found := preemptorsMap[job.Queue]; !found {
 				preemptorsMap[job.Queue] = util.NewPriorityQueue(ssn.JobOrderFn)
 			}
 			preemptorsMap[job.Queue].Push(job)
-			underRequest = append(underRequest, job)
+			// underRequest = append(underRequest, job)


please remove directly

hzxuzhonghu

/lgtm

please address the minor comments in the following pr

Signed-off-by: Thor <[email protected]>

k82cn · 2020-05-30T11:40:32Z

/lgtm

k82cn · 2020-05-30T13:44:06Z

/approve

volcano-sh-bot · 2020-05-30T13:44:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu, k82cn, Thor-wl

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:

~~OWNERS~~ [k82cn]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

volcano-sh-bot requested review from hzxuzhonghu and k82cn May 29, 2020 08:56

volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 29, 2020

Thor-wl force-pushed the master branch from 9a91c2e to 0408f1b Compare May 29, 2020 09:33

k82cn reviewed May 29, 2020

View reviewed changes

Thor-wl force-pushed the master branch from 0408f1b to 3c15b12 Compare May 30, 2020 01:47

k82cn reviewed May 30, 2020

View reviewed changes

pkg/scheduler/actions/preempt/preempt.go Outdated Show resolved Hide resolved

k82cn reviewed May 30, 2020

View reviewed changes

Thor-wl force-pushed the master branch from 3c15b12 to a2213aa Compare May 30, 2020 03:31

Lint:fix lints

2bea0d2

Signed-off-by: Thor <[email protected]>

Thor-wl force-pushed the master branch from a2213aa to 2bea0d2 Compare May 30, 2020 06:07

hzxuzhonghu reviewed May 30, 2020

View reviewed changes

hzxuzhonghu approved these changes May 30, 2020

View reviewed changes

volcano-sh-bot assigned hzxuzhonghu May 30, 2020

volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label May 30, 2020

lint fix and deal with the advice in commit 2bea0d2

286c11d

Signed-off-by: Thor <[email protected]>

volcano-sh-bot removed the lgtm Indicates that a PR is ready to be merged. label May 30, 2020

volcano-sh-bot assigned k82cn May 30, 2020

volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label May 30, 2020

volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 30, 2020

volcano-sh-bot merged commit 2a2145b into volcano-sh:master May 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lint:fix lints #841

Lint:fix lints #841

Thor-wl commented May 29, 2020

volcano-sh-bot commented May 29, 2020

TravisBuddy commented May 29, 2020

TravisBuddy commented May 29, 2020

k82cn May 29, 2020

Thor-wl May 30, 2020

TravisBuddy commented May 30, 2020

k82cn May 30, 2020

hzxuzhonghu May 30, 2020

Thor-wl May 30, 2020

k82cn May 30, 2020

Thor-wl May 30, 2020

k82cn May 30, 2020

Thor-wl May 30, 2020

k82cn commented May 30, 2020

TravisBuddy commented May 30, 2020

hzxuzhonghu May 30, 2020

Thor-wl Jun 1, 2020

hzxuzhonghu May 30, 2020

Thor-wl May 30, 2020

hzxuzhonghu left a comment

k82cn commented May 30, 2020

k82cn commented May 30, 2020

volcano-sh-bot commented May 30, 2020

Lint:fix lints #841

Lint:fix lints #841

Conversation

Thor-wl commented May 29, 2020

volcano-sh-bot commented May 29, 2020

TravisBuddy commented May 29, 2020

Travis tests have failed

TravisBuddy Request Identifier: a94101b0-a18a-11ea-b485-534d9f3e4954

TravisBuddy commented May 29, 2020

Travis tests have failed

TravisBuddy Request Identifier: c40b4690-a18f-11ea-b485-534d9f3e4954

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TravisBuddy commented May 30, 2020

Travis tests have failed

TravisBuddy Request Identifier: e51900b0-a217-11ea-a8e9-e7e13745aead

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k82cn commented May 30, 2020

TravisBuddy commented May 30, 2020

Travis tests have failed

TravisBuddy Request Identifier: 3248f300-a226-11ea-a8e9-e7e13745aead

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hzxuzhonghu left a comment

Choose a reason for hiding this comment

k82cn commented May 30, 2020

k82cn commented May 30, 2020

volcano-sh-bot commented May 30, 2020