Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable new linters #1162

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Enable new linters #1162

wants to merge 7 commits into from

Conversation

ELENAGER
Copy link
Member

No description provided.

@ELENAGER ELENAGER force-pushed the uncomment_linters branch 5 times, most recently from 986b2a5 to daf93fe Compare December 11, 2023 13:58
raghavendra-talur
raghavendra-talur requested changes Dec 11, 2023
View reviewed changes
Copy link
Member

@raghavendra-talur raghavendra-talur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 of the commits look good(errname and removal of deprecated linters).

For the commit that fixes the unused-parameters, I suggest the following:

  1. If the function is a method(or part of a interface), do what you have done.
  2. If it is not a method, then don't use the _ method but remove the parameter from the function signature altogether and change the caller too.

@ELENAGER ELENAGER force-pushed the uncomment_linters branch 3 times, most recently from 13901aa to f5c98d3 Compare December 13, 2023 11:45
raghavendra-talur
raghavendra-talur reviewed Dec 18, 2023
View reviewed changes
controllers/drplacementcontrol_controller_test.go
@@ -191,7 +191,7 @@ func getSyncDRPolicy() *rmn.DRPolicy {
var drstate string

// FakeProgressCallback of function type
func FakeProgressCallback(drpcName string, state string) {
func FakeProgressCallback(_ string, state string) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can remove this parameter.

Copy link
Member Author

@ELENAGER ELENAGER Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It must have specific signature:
// ProgressCallback of function type
type ProgressCallback func(string, string),

controllers/vrg_kubeobjects.go
@@ -168,8 +168,8 @@ func (v *VRGInstance) kubeObjectsCaptureStartOrResumeOrDelay(
}

func kubeObjectsCaptureStartConditionallySecondary(
v *VRGInstance, result *ctrl.Result,
captureStartGeneration int64, captureStartTimeSince, captureStartInterval time.Duration,
v *VRGInstance, _ *ctrl.Result,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can remove these parameters.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function must have a signature:
captureStartConditionally func(*VRGInstance, *ctrl.Result, int64, time.Duration, time.Duration, func())

controllers/vrg_kubeobjects.go
@@ -187,7 +187,7 @@ func kubeObjectsCaptureStartConditionallySecondary(

func kubeObjectsCaptureStartConditionallyPrimary(
v *VRGInstance, result *ctrl.Result,
captureStartGeneration int64, captureStartTimeSince, captureStartInterval time.Duration,
_ int64, captureStartTimeSince, captureStartInterval time.Duration,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function must have a signature:
captureStartConditionally func(*VRGInstance, *ctrl.Result, int64, time.Duration, time.Duration, func())

controllers/vrg_volrep_test.go
@@ -1788,14 +1788,14 @@ func pvcUnprotectedVerify(
type pvcPreDeleteVerify func(ramendrv1alpha1.VolumeReplicationGroup, types.NamespacedName, string)

func vrgPvcStatusAbsentVerify(
vrg ramendrv1alpha1.VolumeReplicationGroup, pvcNamespacedName types.NamespacedName, pvName string,
vrg ramendrv1alpha1.VolumeReplicationGroup, pvcNamespacedName types.NamespacedName, _ string,
) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function must have signature:
type pvcPreDeleteVerify func(ramendrv1alpha1.VolumeReplicationGroup, types.NamespacedName, string)

controllers/vrg_volrep_test.go
) {
By("not storing PVC in VRG's status")
Expect(vrgController.FindProtectedPVC(&vrg, pvcNamespacedName.Namespace, pvcNamespacedName.Name)).To(BeNil())
}

func pvcClusterDataProtectedFalseVerify(
vrg ramendrv1alpha1.VolumeReplicationGroup, pvcNamespacedName types.NamespacedName, pvName string,
vrg ramendrv1alpha1.VolumeReplicationGroup, pvcNamespacedName types.NamespacedName, _ string,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function must have signature:
type pvcPreDeleteVerify func(ramendrv1alpha1.VolumeReplicationGroup, types.NamespacedName, string)

controllers/vrg_volrep_test.go
@@ -1835,7 +1835,7 @@ func pvcClusterDataProtectedStatusVerify(

type pvcPostDeleteVerify func(types.NamespacedName, string)

func vrAndPvcDeletionTimestampsRecentVerify(pvcNamespacedName types.NamespacedName, pvName string) {
func vrAndPvcDeletionTimestampsRecentVerify(pvcNamespacedName types.NamespacedName, _ string) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function must have signature:
type pvcPreDeleteVerify func(ramendrv1alpha1.VolumeReplicationGroup, types.NamespacedName, string)

controllers/drcluster_controller.go Outdated
@@ -449,7 +449,7 @@ func validateCIDRsFormat(drcluster *ramen.DRCluster, log logr.Logger) error {
return nil
}

func (r DRClusterReconciler) processDeletion(u *drclusterInstance) (ctrl.Result, error) {
func processDeletion(u *drclusterInstance) (ctrl.Result, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep this as a method. You can probably remove the receiver like you have in other cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

raghavendra-talur
raghavendra-talur reviewed Dec 18, 2023
View reviewed changes
controllers/drplacementcontrol.go Outdated
homeCluster := ""
homeClusterNamespace := ""
homeCluster = ""
homeClusterNamespace = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can remove these lines as null value of string is empty string in go.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@ELENAGER ELENAGER force-pushed the uncomment_linters branch 3 times, most recently from 36d3378 to 1544fe5 Compare December 19, 2023 10:04
@ELENAGER ELENAGER force-pushed the uncomment_linters branch 2 times, most recently from 935ac2e to ed46af5 Compare December 27, 2023 12:11
@ELENAGER ELENAGER force-pushed the uncomment_linters branch 4 times, most recently from 74306f0 to ebc2060 Compare January 22, 2024 13:30
@ELENAGER
Enable errname linter - checks that errors are prefixed with the Err
bb7df42 
Signed-off-by: Elena Gershkovich <elenage@il.ibm.com>
@ELENAGER
Remove deprecated linter flags
b06e807 
Signed-off-by: Elena Gershkovich <elenage@il.ibm.com>
@ELENAGER
Enable unused-parameter revive flag
6cf6325 
Signed-off-by: Elena Gershkovich <elenage@il.ibm.com>
@ELENAGER
Enable unused-receiver revive flag
836cc93 
Signed-off-by: Elena Gershkovich <elenage@il.ibm.com>
@ELENAGER
Enable confusing-results revive flag
d466d29 
Signed-off-by: Elena Gershkovich <elenage@il.ibm.com>
@ELENAGER
Enable bool-literal-in-expr revive flag
9cb41cc 
Signed-off-by: Elena Gershkovich <elenage@il.ibm.com>
@ELENAGER
Enable confusing-naming revive flag
6452c50 
Signed-off-by: Elena Gershkovich <elenage@il.ibm.com>
@ELENAGER ELENAGER force-pushed the uncomment_linters branch 3 times, most recently from 1841da2 to 6452c50 Compare January 25, 2024 10:53
@ShyamsundarR ShyamsundarR mentioned this pull request Mar 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raghavendra-talur raghavendra-talur Awaiting requested review from raghavendra-talur

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ELENAGER @raghavendra-talur