Skip to content

Conversation

appleboy
Copy link
Member

  1. var a => a:
  2. //xxxx => // xxx

@appleboy appleboy added this to the 1.14.0 milestone Feb 20, 2021
@techknowlogick techknowlogick added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Feb 20, 2021
@zeripath
Copy link
Contributor

Ironically for a linting pr this one has caused a linting failure...

Following this pr are you able to remove any special exemptions for models/ from golangcilint?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 20, 2021
@6543
Copy link
Member

6543 commented Feb 23, 2021

1. `var a` => `a:`

2. `//xxxx` => `// xxx`

2. I can understand ...

but what reason/impact do 1. have?

@appleboy

@zeripath
Copy link
Contributor

it's more a stylistic issue. In general in our code we use a := instead of var a =

@6543 6543 removed this from the 1.14.0 milestone Mar 2, 2021
@6543
Copy link
Member

6543 commented Mar 2, 2021

@appleboy pull got conflicts & CI still failing :/

@appleboy
Copy link
Member Author

appleboy commented Mar 2, 2021

@6543 I will take it.

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@appleboy
Copy link
Member Author

@6543 Done. Please help to review.

@lunny
Copy link
Member

lunny commented Mar 13, 2021

  1. var a => a:
  2. //xxxx => // xxx

I like var a = than a := better :)

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 13, 2021
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 14, 2021
@codecov-io
Copy link

Codecov Report

Merging #14754 (332090f) into master (164e35e) will decrease coverage by 0.00%.
The diff coverage is 74.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14754      +/-   ##
==========================================
- Coverage   42.24%   42.23%   -0.01%     
==========================================
  Files         774      774              
  Lines       83008    83011       +3     
==========================================
- Hits        35068    35061       -7     
- Misses      42243    42252       +9     
- Partials     5697     5698       +1     
Impacted Files Coverage Δ
models/admin.go 60.31% <ø> (ø)
models/commit_status.go 61.74% <0.00%> (ø)
models/error.go 40.60% <ø> (ø)
models/fixture_generation.go 70.00% <ø> (ø)
models/helper_environment.go 93.33% <ø> (ø)
models/issue_lock.go 0.00% <0.00%> (ø)
models/issue_tracked_time.go 69.93% <ø> (ø)
models/lfs.go 22.64% <0.00%> (ø)
models/lfs_lock.go 56.00% <ø> (ø)
models/migrate.go 0.00% <0.00%> (ø)
... and 79 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 164e35e...332090f. Read the comment docs.

@6543 6543 merged commit 167b0f4 into go-gitea:master Mar 14, 2021
@appleboy appleboy deleted the format branch March 15, 2021 06:08
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/code-linting type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants