karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.11k stars 805 forks source link

[umbrella] Enable golangci-lint(revive) to improve code quality #4490

Closed jwcesign closed 3 months ago

jwcesign commented 3 months ago

What would you like to be added:

Enable the appropriate revive check rules to improve PR quality.

Why is this needed:

For golangci-lint (revive), in the absence of any specific configuration, default checking rules will be applied as reference: https://github.com/golangci/golangci-lint/blob/0264eaa5c7851a35883e2f462e66d648ab8598df/pkg/golinters/revive.go#L251

In PR https://github.com/karmada-io/karmada/pull/3567, a single rule is configured as shown in the image:

image.

This configuration results in only this rule being used and disables checking for other rules.

Thus, it's advisable to activate the default rules and rectify the associated warnings(You can pick one or multiple):

Check out the rule details here: https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#if-return You can update this link: https://github.com/karmada-io/karmada/blob/c218f93ce3fe6a626f37b09b418e687d4bf9b41e/.golangci.yml#L67, then run hack/verify-staticcheck.sh to view the related warning.

Node: Task decomposition will be updated in a timely manner as the subsequent development progresses.

jwcesign commented 3 months ago

/label good first issue

RainbowMango commented 3 months ago

/lable good-first-issue

zhzhuang-zju commented 3 months ago

@Affan-7 Hi~Are you interested in getting involved?

Affan-7 commented 3 months ago

Thanks @zhzhuang-zju

I would love to work on this.

Affan-7 commented 3 months ago

/assign

jwcesign commented 3 months ago

Hi, @Affan-7 Thanks. Which one would you like to work on? I can assign to you.

liangyuanpeng commented 3 months ago

I will take the SuperfluousElse, assign to me please,Thanks.

zhzhuang-zju commented 3 months ago

I will take the PackageComments and DotImports, assign to me please,Thanks. BTW, if necessary, you can also assign other tasks to me.

chaosi-zju commented 3 months ago
Larry-shuo commented 3 months ago

I want to try this issue, can I take some tasks? THX!

zhzhuang-zju commented 3 months ago

I want to try this issue, can I take some tasks? THX!

Of course, very welcome~ You can choose to implement the task assigned to @jwcesign above but which has not yet implemented PR. It would be better if you could tell us the tasks you selected, so as to avoid duplication of operations.

Larry-shuo commented 3 months ago

I will take Range and ErrorNaming, assign to me please, Thanks.

zhzhuang-zju commented 3 months ago

I will take Range and ErrorNaming, assign to me please, Thanks.

Okay, you can start it first, and we will modify the task information later~

sagnik3788 commented 3 months ago

can i take the TimeNaming and EmptyBlock, Thanks.

jwcesign commented 3 months ago

can i take the TimeNaming and EmptyBlock , Thanks.

Hi, @XiShanYongYe-Chang Is that ok for you?

XiShanYongYe-Chang commented 3 months ago

Hi, @XiShanYongYe-Chang Is that ok for you?

Offcrouse~.

XiShanYongYe-Chang commented 3 months ago

Kindly ping @sagnik3788

Affan-7 commented 3 months ago

Dear contributors,

Please assign me some more issues. I would love to work on the Karmada.

Affan-7 commented 3 months ago

I think we should implement a triage/accepted label. It would help new contributors a lot.

RainbowMango commented 3 months ago

I think we should implement a triage/accepted label. It would help new contributors a lot.

How does it help? Do we need to upgrade prow?

Affan-7 commented 3 months ago

How does it help? Do we need to upgrade prow?

It would help new contributors to work on things that matter. It will avoid time wasted on something that the community doesn't needs. A new contributor will avoid issues that don't have traige/accepted label.

I think we don't need to upgrade the prow, we just need these labels added to the Karmada repo like Kubernetes.

https://github.com/kubernetes/website/labels?q=triage

RainbowMango commented 3 months ago

Thanks @Affan-7 I found this guide for traige: https://www.kubernetes.dev/docs/guide/issue-triage/ I don't know if we need this for now. I will look into it further.

RainbowMango commented 3 months ago

@sagnik3788 @jwcesign Given no response from @sagnik3788, I finished the left two tasks at #4568 during the update of golangci-lint. But still thanks to @sagnik3788.