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

Resolve the linter issue. #7707

Closed
wants to merge 1 commit into from

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Apr 19, 2024

Thank you for contributing to Velero!

Please add a summary of your change

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.

@blackpiglet blackpiglet added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Apr 19, 2024
Copy link

codecov bot commented Apr 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.49%. Comparing base (f04fbbc) to head (48301e7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7707   +/-   ##
=======================================
  Coverage   58.49%   58.49%           
=======================================
  Files         343      343           
  Lines       28486    28486           
=======================================
  Hits        16664    16664           
  Misses      10406    10406           
  Partials     1416     1416           

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

Signed-off-by: Xun Jiang <[email protected]>
@blackpiglet blackpiglet marked this pull request as ready for review April 19, 2024 11:09
@blackpiglet blackpiglet marked this pull request as draft April 19, 2024 11:10
@mmorel-35
Copy link
Contributor

Hi @blackpiglet ,
.golangci apart how is it different from #7595 ?

Would you merge my PR and then complete it with this one ?

@@ -44,7 +35,7 @@ run:
# output configuration options
output:
# colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
format: colored-line-number
formats: colored-line-number
Copy link
Contributor

@mmorel-35 mmorel-35 Apr 19, 2024

Choose a reason for hiding this comment

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

Suggested change
formats: colored-line-number
formats:
- format: colored-line-number
path: stdout

@blackpiglet
Copy link
Contributor Author

blackpiglet commented Apr 20, 2024

Hi @blackpiglet , .golangci apart how is it different from #7595 ?

Would you merge my PR and then complete it with this one ?

Thanks for your contribution, and sorry for not realizing your PR's value earlier.
I agree #7595 should be merged.

At first, I thought the linter action didn't print the issued line number and the reported linter is due to the configuration file name.
I figured out that the default behavior of the action is printing that information as annotations.

@mmorel-35
Copy link
Contributor

mmorel-35 commented Apr 20, 2024

sorry for not realizing your PR's value earlier.

No worries, things aren’t always obvious 🤗.

@mateusoliveira43
Copy link
Contributor

related work #7194 (I will be able to work on that PR again only on may 13)

@blackpiglet
Copy link
Contributor Author

Close because the fix is already merged with #7697

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-unit-tests 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.

3 participants