-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Code Formats, Nits & Unused Func/Var deletions #15286
Conversation
6543
commented
Apr 5, 2021
- _ to unused func options
- rm useless brakets
- rm trifial non used models functions
- rm dead code
- rm dead global vars
- fix routers/api/v1/repo/issue.go
- dont overload import module
if m == nil { | ||
return nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because of dead code
see L100 & L110
If we like to keep it, we need a reason we should write down here ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lunny do we have a reason to keep it?
Maybe some of these can be enforced by the linter. |
@@ -176,7 +175,7 @@ func SearchIssues(ctx *context.APIContext) { | |||
|
|||
// Only fetch the issues if we either don't have a keyword or the search returned issues | |||
// This would otherwise return all issues if no issues were found by the search. | |||
if len(keyword) == 0 || len(issueIDs) > 0 || len(labelIDs) > 0 { | |||
if len(keyword) == 0 || len(issueIDs) > 0 || len(includedLabelNames) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This maybe a bug but not a refactor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't dig into it yet - posible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look's like in an edge case the label filter potentially is not applied, but I could not hit it, so It's a code improvement to prevent becoming a bug in the future
🚀 |