Skip to content

Conversation

@lunny
Copy link
Member

@lunny lunny commented Feb 18, 2017

No description provided.

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Feb 18, 2017
@lunny lunny added this to the 1.1.0 milestone Feb 18, 2017
type IssueList []*Issue

func (issues IssueList) getRepoIDs() []int64 {
repoIDs := make([]int64, 0, len(issues))
Copy link
Member

@ethantkoenig ethantkoenig Feb 19, 2017

Choose a reason for hiding this comment

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

I would suggest doing something like

repoIDs := make(map[int64]bool, len(issues))
for _, issue := range issues {
  repoIDs[issue.RepoID] = true
}
return keys(repoIDs) // keys is a helper function (map[int64]bool) -> []int64

This is more efficient, and more concise (since the keys helper function can be shared across the getSomethingIDs functions)

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@lunny
Copy link
Member Author

lunny commented Feb 20, 2017

This PR is depended on #987.

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

LGTM

@tboerger tboerger 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 Feb 20, 2017
@appleboy
Copy link
Member

LGTM

@tboerger tboerger 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 Feb 22, 2017
@lunny lunny merged commit 1f7837d into go-gitea:master Feb 22, 2017
@lunny lunny deleted the lunny/refactor_issuelist_loadattributes branch April 19, 2017 05:43
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
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. 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.

4 participants