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

chore: Fix some code formats #968

Merged
merged 3 commits into from
Apr 21, 2022
Merged

Conversation

Gallardot
Copy link
Member

Type of change:

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What this PR does / why we need it:

ref:CodeReviewComment. Fix some code formats. IDE will not remind.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@codecov-commenter
Copy link

Codecov Report

Merging #968 (d6a5b7b) into master (22cfb5e) will not change coverage.
The diff coverage is n/a.

❗ Current head d6a5b7b differs from pull request most recent head 68f2539. Consider uploading reports for the commit 68f2539 to get more accurate results

@@           Coverage Diff           @@
##           master     #968   +/-   ##
=======================================
  Coverage   31.75%   31.75%           
=======================================
  Files          72       72           
  Lines        7935     7935           
=======================================
  Hits         2520     2520           
  Misses       5139     5139           
  Partials      276      276           
Impacted Files Coverage Δ
cmd/ingress/ingress.go 78.43% <ø> (ø)
pkg/api/router/apisix_healthz.go 66.66% <ø> (ø)
pkg/apisix/consumer.go 36.73% <ø> (ø)
pkg/apisix/global_rule.go 36.48% <ø> (ø)
pkg/apisix/noop.go 0.00% <ø> (ø)
pkg/apisix/resource.go 60.27% <ø> (ø)
pkg/apisix/stream_route.go 36.42% <ø> (ø)
pkg/apisix/upstream.go 37.24% <ø> (ø)
pkg/config/config.go 64.64% <ø> (ø)
pkg/ingress/apisix_cluster_config.go 0.00% <ø> (ø)
... and 41 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77ab065...68f2539. Read the comment docs.

@SkyeYoung
Copy link
Member

Shouldn't code format be handled by lint or format tools? 🤔

@tao12345666333
Copy link
Member

Thank you for your contribution, I checked golang's documentation and haven't found any instructions about having to have a blank line after the license header file.

Can you explain the need for this modification?

@tokers
Copy link
Contributor

tokers commented Apr 18, 2022

Shouldn't code format be handled by lint or format tools? 🤔

Yes, Code format should be handled by the gofmt tool.

@tao12345666333
Copy link
Member

In fact, the code in the current repo has passed the gofmt check

@tokers
Copy link
Contributor

tokers commented Apr 19, 2022

In fact, the code in the current repo has passed the gofmt check

@Gallardot Could you revert these parts?

Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

Thanks!

@Gallardot
Copy link
Member Author

Thank you for your contribution, I checked golang's documentation and haven't found any instructions about having to have a blank line after the license header file.

Can you explain the need for this modification?

It's just that some files have a newline after the license header and some don't. Just to keep the code in a consistent format.

Shouldn't code format be handled by lint or format tools? 🤔

Agree, maybe current tools don't cover this detail. Maybe we can make some adjustments back there.

@tao12345666333 tao12345666333 merged commit 4a0fc0c into apache:master Apr 21, 2022
@tao12345666333
Copy link
Member

It's just that some files have a newline after the license header and some don't. Just to keep the code in a consistent format.

If we are modifying a feature in a file, I think it is acceptable to make adjustments to it in passing.
But it would not be reasonable to make a centralized change to it specifically in a PR.

@Gallardot Gallardot deleted the format branch April 21, 2022 02:37
AlinsRan pushed a commit to AlinsRan/apisix-ingress-controller that referenced this pull request May 9, 2022
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.

None yet

5 participants