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

organize golangci workflow #7595

Merged
merged 1 commit into from
May 16, 2024

Conversation

mmorel-35
Copy link
Contributor

@mmorel-35 mmorel-35 commented Mar 29, 2024

Please add a summary of your change

  • check out before go setup
  • use go version from go.mod
  • define output format in golangci

Does your change fix a particular issue?

Fixes #(issue)

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@mmorel-35 mmorel-35 marked this pull request as ready for review March 29, 2024 19:51
Copy link

codecov bot commented Mar 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.66%. Comparing base (27392d3) to head (bc1e88c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7595   +/-   ##
=======================================
  Coverage   58.66%   58.66%           
=======================================
  Files         345      345           
  Lines       28733    28733           
=======================================
  Hits        16856    16856           
  Misses      10448    10448           
  Partials     1429     1429           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kaovilai
Copy link
Member

conflicts with #7194

@mmorel-35
Copy link
Contributor Author

mmorel-35 commented Mar 29, 2024

Yes, it’s on purpose because with a rebase after this one is merged the modification done in 7194 concering golangci-lint config are going to be easier to review

Copy link
Contributor

@mateusoliveira43 mateusoliveira43 left a comment

Choose a reason for hiding this comment

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

I can rebase my PR afterwards

@blackpiglet blackpiglet added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Apr 1, 2024
blackpiglet
blackpiglet previously approved these changes Apr 1, 2024
blackpiglet
blackpiglet previously approved these changes Apr 1, 2024
@mmorel-35
Copy link
Contributor Author

Sorry, the rebase dismissed your reviews

@anshulahuja98
Copy link
Collaborator

CI failed due to throttling, rerunning it.

anshulahuja98
anshulahuja98 previously approved these changes Apr 18, 2024
@mmorel-35 mmorel-35 requested a review from blackpiglet April 18, 2024 06:21
Copy link
Contributor

@blackpiglet blackpiglet left a comment

Choose a reason for hiding this comment

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

I suggest not to change the configuration file name.
I'm trying to use golangci/golangci-lint-action@v4 to replace the make lint in the lint action.
And the GitHub action cannot find files those names start with ..

@mmorel-35
Copy link
Contributor Author

.golangci.yaml is one of the default file handled by golangci-lint so you won't need to customize the golangci config file, it will work out of the box, I can complete this PR to package it if you want ?

@blackpiglet
Copy link
Contributor

.golangci.yaml is one of the default file handled by golangci-lint so you won't need to customize the golangci config file, it will work out of the box, I can complete this PR to package it if you want ?

Please take a view of this commit.
abb1140

The linter's output is not formatted if it doesn't pass the configuration file parameter.
If the configuration file parameter is specified, the action always complains ".golangci.yaml" cannot be found.

@mmorel-35 mmorel-35 force-pushed the golangci-lint-config branch 2 times, most recently from ba0289d to b76dbc8 Compare April 18, 2024 07:52
@mmorel-35
Copy link
Contributor Author

See this :
image

When you use one of the expected file names, the config file is automatically detected

@mmorel-35 mmorel-35 requested a review from blackpiglet April 18, 2024 07:56
@mmorel-35 mmorel-35 force-pushed the golangci-lint-config branch from e4cff67 to a0c1431 Compare April 18, 2024 20:00
@mmorel-35 mmorel-35 mentioned this pull request Apr 19, 2024
3 tasks
blackpiglet
blackpiglet previously approved these changes Apr 20, 2024
@mmorel-35 mmorel-35 force-pushed the golangci-lint-config branch 2 times, most recently from 11659e9 to 151b622 Compare April 22, 2024 08:52
blackpiglet
blackpiglet previously approved these changes Apr 22, 2024
@mmorel-35 mmorel-35 force-pushed the golangci-lint-config branch 3 times, most recently from ab21689 to 70a46ac Compare May 5, 2024 07:15
@mmorel-35 mmorel-35 force-pushed the golangci-lint-config branch 2 times, most recently from 6aee83f to 42990bf Compare May 10, 2024 07:31
@mmorel-35 mmorel-35 requested a review from blackpiglet May 10, 2024 07:34
@mmorel-35 mmorel-35 changed the title rename golangci-lint config file organize golangci workflow May 13, 2024
@mmorel-35
Copy link
Contributor Author

@blackpiglet ,
Appart the two required approvals, anything blocking this to be merged ?

blackpiglet
blackpiglet previously approved these changes May 14, 2024
@blackpiglet
Copy link
Contributor

We need the maintainers' approval to merge the PR.
Involve more people to accelerate the process.

@blackpiglet blackpiglet merged commit a0b7382 into vmware-tanzu:main May 16, 2024
66 checks passed
@mmorel-35 mmorel-35 deleted the golangci-lint-config branch May 16, 2024 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants