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.

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

feat: support for checks of rollback and restart #204

Merged
merged 7 commits into from
Dec 11, 2023

Conversation

xuzhu-591
Copy link
Collaborator

Description of your changes

Fixes #

I have:

  • Read and followed Horizon's contribution process.
  • Run make build && make lint to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

@xuzhu-591 xuzhu-591 force-pushed the feat-approval branch 2 times, most recently from 1f3ecb7 to 7c45cdd Compare November 28, 2023 06:06
@@ -1759,6 +1778,66 @@ func (g *clusterGitopsRepo) readFile(ctx context.Context, application, cluster,
return g.gitlabLib.GetFile(ctx, pid, GitOpsBranch, fileName)
}

// for internal usage
// manifestVersionChanged determines whether manifest version is changed
func (g *clusterGitopsRepo) manifestVersionChanged(ctx context.Context, application,
Copy link

Choose a reason for hiding this comment

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

Method clusterGitopsRepo.manifestVersionChanged has 5 return statements (exceeds 4 allowed).

return &service{
applicationService: applicationSvc,
clusterManager: manager.ClusterMgr,
func (s service) SyncDBWithGitRepo(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method service.SyncDBWithGitRepo has 6 return statements (exceeds 4 allowed).

return nil
}

func (c *controller) executeRollback(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeRollback has 17 return statements (exceeds 4 allowed).

return nil
}

func (c *controller) executeRestart(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeRestart has 7 return statements (exceeds 4 allowed).

}
}

func (c *controller) executeDeploy(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeDeploy has 9 return statements (exceeds 4 allowed).

return nil
}

func (c *controller) executeRollback(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeRollback has 94 lines of code (exceeds 50 allowed). Consider refactoring.

}
}

func (c *controller) executeDeploy(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeDeploy has 71 lines of code (exceeds 50 allowed). Consider refactoring.

}
}

func (c *controller) executeDeploy(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeDeploy has 5 arguments (exceeds 4 allowed). Consider refactoring.

@xuzhu-591 xuzhu-591 force-pushed the feat-approval branch 2 times, most recently from 2c688de to a8f5d16 Compare December 4, 2023 12:08
@xuzhu-591 xuzhu-591 force-pushed the feat-approval branch 4 times, most recently from 3dfe398 to 036c679 Compare December 7, 2023 08:43
return nil
}

func (c *controller) executeRollback(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeRollback has 88 lines of code (exceeds 50 allowed). Consider refactoring.

}
}

func (c *controller) executeDeploy(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeDeploy has 67 lines of code (exceeds 50 allowed). Consider refactoring.

Signed-off-by: xu.zhu <[email protected]>
return nil
}

func (c *controller) executeRollback(ctx context.Context, application *appmodels.Application,
Copy link

Choose a reason for hiding this comment

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

Method controller.executeRollback has 87 lines of code (exceeds 50 allowed). Consider refactoring.

Copy link

codeclimate bot commented Dec 11, 2023

Code Climate has analyzed commit 7b78016 and detected 16 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 10
Duplication 6

The test coverage on the diff in this pull request is 48.6% (60% is the threshold).

This pull request will bring the total coverage in the repository to 66.2% (0.2% change).

View more on Code Climate.

@xuzhu-591 xuzhu-591 merged commit aa5e0fa into horizoncd:main Dec 11, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants