refactor: append, build variable and type switch #4940

appleboy · 2018-09-15T05:43:58Z

append chains to the same slice that can be done in a single append call
when predeclared identifiers shadowed in assignments
loops that copy big objects during each iteration
type switches that can benefit from type guard clause with a variable

lafriks · 2018-09-15T12:51:03Z

Blocked till 1.6.0-rc1 is released

techknowlogick · 2018-09-16T23:48:38Z

models/user.go

@@ -14,6 +14,7 @@ import (
 "errors"
 "fmt"
 "image"
+


See a4f370b

stale · 2019-02-17T14:02:18Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

lafriks · 2019-02-17T14:19:29Z

Please resolve conflicts and sorry for forgetting to review it

stale · 2019-04-20T14:59:51Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

lafriks · 2019-04-20T21:20:14Z

@appleboy conflicts

appleboy · 2019-05-28T08:32:44Z

@lafriks resolved conflicts.

zeripath · 2019-05-28T09:02:58Z

modules/base/tool.go

 isInt = false
 }

- switch right := right.(type) {
+ switch v := right.(type) {


Could we not refactor this further to get rid of the code duplication completely?

lunny · 2019-05-28T09:06:21Z

models/issue_assignees_test.go

@@ -43,8 +43,7 @@ func TestUpdateAssignee(t *testing.T) {
 assert.NoError(t, err)

 var expectedAssignees []*User
- expectedAssignees = append(expectedAssignees, user2)
- expectedAssignees = append(expectedAssignees, user3)
+ expectedAssignees = append(expectedAssignees, user2, user3)


~~I think this may changed the tests?~~

@lunny I don't know what do you mean?

codecov-io · 2019-05-28T09:13:37Z

Codecov Report

Merging #4940 into master will decrease coverage by <.01%.
The diff coverage is 56.66%.

@@            Coverage Diff             @@
##           master    #4940      +/-   ##
==========================================
- Coverage   41.43%   41.43%   -0.01%     
==========================================
  Files         442      442              
  Lines       59693    59692       -1     
==========================================
- Hits        24735    24733       -2     
- Misses      31721    31722       +1     
  Partials     3237     3237

Impacted Files	Coverage Δ
routers/user/auth_openid.go	`0% <0%> (ø)`	⬆️
models/migrations/v64.go	`0% <0%> (ø)`	⬆️
modules/templates/helper.go	`47.48% <0%> (+0.13%)`	⬆️
modules/base/tool.go	`72.54% <100%> (ø)`	⬆️
models/user.go	`51.45% <33.33%> (ø)`	⬆️
modules/log/colors_router.go	`83.33% <0%> (-4.17%)`	⬇️
modules/log/event.go	`64.46% <0%> (-1.53%)`	⬇️
models/repo_list.go	`73.09% <0%> (+1.01%)`	⬆️

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 31557b1...37b81ed. Read the comment docs.

* refactor: append, build variable and type switch * fix: remove redundant space.

appleboy added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Sep 15, 2018

appleboy added this to the 1.6.0 milestone Sep 15, 2018

lafriks modified the milestones: 1.6.0, 1.7.0 Sep 15, 2018

lafriks added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Sep 15, 2018

bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 15, 2018

techknowlogick reviewed Sep 16, 2018

View reviewed changes

lunny removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Oct 18, 2018

techknowlogick modified the milestones: 1.7.0, 1.8.0 Dec 19, 2018

stale bot added the issue/stale label Feb 17, 2019

stale bot removed the issue/stale label Feb 17, 2019

techknowlogick modified the milestones: 1.8.0, 1.9.0 Feb 19, 2019

stale bot added the issue/stale label Apr 20, 2019

stale bot removed the issue/stale label Apr 20, 2019

appleboy added 2 commits May 28, 2019 16:30

refactor: append, build variable and type switch

c19281a

fix: remove redundant space.

391e490

appleboy force-pushed the refactor2 branch from 992f717 to 391e490 Compare May 28, 2019 08:32

appleboy requested review from lafriks, lunny and techknowlogick May 28, 2019 08:33

zeripath reviewed May 28, 2019

View reviewed changes

zeripath approved these changes May 28, 2019

View reviewed changes

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 May 28, 2019

lunny requested changes May 28, 2019

View reviewed changes

sapk approved these changes May 28, 2019

View reviewed changes

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 May 28, 2019

lunny approved these changes May 28, 2019

View reviewed changes

Merge branch 'master' into refactor2

37b81ed

appleboy merged commit 743697a into go-gitea:master May 28, 2019

appleboy deleted the refactor2 branch May 28, 2019 15:45

jeffliu27 pushed a commit to jeffliu27/gitea that referenced this pull request Jul 18, 2019

refactor: append, build variable and type switch (go-gitea#4940)

fe2ba3d

* refactor: append, build variable and type switch * fix: remove redundant space.

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: append, build variable and type switch #4940

refactor: append, build variable and type switch #4940

appleboy commented Sep 15, 2018

lafriks commented Sep 15, 2018

techknowlogick Sep 16, 2018

appleboy Sep 17, 2018

stale bot commented Feb 17, 2019

lafriks commented Feb 17, 2019

stale bot commented Apr 20, 2019

lafriks commented Apr 20, 2019

appleboy commented May 28, 2019

zeripath May 28, 2019

lunny May 28, 2019 •

edited

Loading

appleboy May 28, 2019

codecov-io commented May 28, 2019 •

edited

Loading

refactor: append, build variable and type switch #4940

refactor: append, build variable and type switch #4940

Conversation

appleboy commented Sep 15, 2018

lafriks commented Sep 15, 2018

techknowlogick Sep 16, 2018

Choose a reason for hiding this comment

appleboy Sep 17, 2018

Choose a reason for hiding this comment

stale bot commented Feb 17, 2019

lafriks commented Feb 17, 2019

stale bot commented Apr 20, 2019

lafriks commented Apr 20, 2019

appleboy commented May 28, 2019

zeripath May 28, 2019

Choose a reason for hiding this comment

lunny May 28, 2019 • edited Loading

Choose a reason for hiding this comment

appleboy May 28, 2019

Choose a reason for hiding this comment

codecov-io commented May 28, 2019 • edited Loading

Codecov Report

lunny May 28, 2019 •

edited

Loading

codecov-io commented May 28, 2019 •

edited

Loading