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

golangci-lint #1319

Merged
merged 18 commits into from
Aug 21, 2024
Merged

golangci-lint #1319

merged 18 commits into from
Aug 21, 2024

Conversation

ale8k
Copy link
Contributor

@ale8k ale8k commented Aug 19, 2024

Description

Introduces golangci-lint action. Now all of JIMM's linting issues have been fixes. This should pass.

Engineering checklist

Check only items that apply

  • Documentation updated
  • Covered by unit tests
  • Covered by integration tests

Test instructions

Notes for code reviewers

@ale8k ale8k requested a review from a team as a code owner August 19, 2024 12:40
.github/workflows/golangci-lint.yaml Outdated Show resolved Hide resolved
cmd/jaas/cmd/addserviceaccount_test.go Outdated Show resolved Hide resolved
kian99
kian99 previously approved these changes Aug 21, 2024
@@ -9,7 +9,7 @@ run:
tests: true
allow-parallel-runners: false
allow-serial-runners: false
# go: "1.17" # Do not set a go limit
# go: "1.23"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this comment stick around?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't think it does any harm really

uses: actions/setup-go@v5
with:
go-version: stable
cache: false
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose we can try turn this on in a follow-up PR and see what effect it has?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah definitely!

Copy link
Contributor

@kian99 kian99 left a comment

Choose a reason for hiding this comment

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

Could we also create a make target for running linting please.

@ale8k ale8k merged commit 970ec0c into canonical:v3 Aug 21, 2024
4 checks passed
kian99 pushed a commit to kian99/jimm that referenced this pull request Sep 3, 2024
* golangci-lint

* test linter with err

* will this work?

* perhaps it is working?

* try a 5 min timeout with verbose...

* 10m no critic

* try a bump

* wip

* reference field directly?

* tmate

* bump jimm?

* approle

* .

* int overflows.

* rename

* extra newline

* add linting to make

* make target
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.

3 participants