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

Add gh linter - fix issues in kubeops, asset-syncer and apprepo-ctrl #5060

Merged
merged 3 commits into from
Jul 12, 2022

Conversation

antgamdia
Copy link
Contributor

Description of the change

This PR is aimed at fixing those issues in the kubeops, asset-syncerand apprepo-ctrl components. Some of them has been fixed and other, just marked as nolint.

Benefits

Once we fix every issue, we will be able to merge the PR and therefore detect any linter issue before merging a PR.

Possible drawbacks

N/A

Applicable issues

Additional information

N/A

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Comment on lines +14 to +21
ARG GOLANGCILINT_VERSION="1.46.2"

RUN curl -sfL https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v$GOSEC_VERSION
RUN go install github.com/golangci/golangci-lint/cmd/golangci-lint@v$GOLANGCILINT_VERSION

# Run golangci-lint to detect issues
RUN golangci-lint run --timeout=10m ./cmd/kubeops/...
RUN golangci-lint run --timeout=10m ./pkg/...
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why investing effort in Kubeops if will be removed (hopefully) soon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mainly for consistency, but also, if want the github action to be run successfully, we need to fix any issue in the existing codebase. So, given that we would be removing the lining issues from Kubeops, why not add the linter in the building stage, as in the rest of go images?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not keen on investing effort in soon-to-be-dead code. Isn't it possible to exclude Kubeops part from any linting action?

@antgamdia antgamdia merged commit 9e6909e into addGHLinter Jul 12, 2022
@antgamdia antgamdia deleted the addGHLinter-rest branch July 12, 2022 12:20
antgamdia added a commit that referenced this pull request Jul 15, 2022
* Add golangci-lint gh action

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Increase timeout

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Remove wrongly commited file

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Add linter to dockerfile

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Add gh linter - fix issues in kubeops, asset-syncer and apprepo-ctrl (#5060)

* Fix issues in kubeops

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Fix issues in assety-syncer

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Fix isses in apprepository-controller

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Fix linter issues (#5059)

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Add gh linter - fix issues in kubeappsapis (#5058)

* Fix linter issues in test files

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Fix linter issues

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Fix issues

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Fix errata

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Fix lint issues and tests

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Comment out line
as it is just being used by an in-progress unit test

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>

* Fix test case

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants