-
Notifications
You must be signed in to change notification settings - Fork 58
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
Integrate GolangCI-Lint #108
Conversation
func (h *Handler) handleErrorWithCode(w http.ResponseWriter, code int, errTitle string, err error) { | ||
w.Header().Set("Content-Type", "application/json") | ||
w.WriteHeader(code) | ||
b, _ := json.Marshal(struct { | ||
Error string `json:"error"` | ||
Details string `json:"details"` | ||
}{ | ||
Error: errTitle, | ||
Details: err.Error(), | ||
}) | ||
_, _ = w.Write(b) | ||
} | ||
|
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.
Unused code
func (p *Plugin) MessageWillBeUpdated(c *plugin.Context, post *model.Post, _ *model.Post) (*model.Post, string) { | ||
conf := p.getConfig() | ||
if conf.EnableOnUpdate { | ||
return p.ProcessPost(c, post) | ||
} else { | ||
if !conf.EnableOnUpdate { | ||
return post, "" | ||
} | ||
|
||
return p.ProcessPost(c, post) |
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.
Invested the logic here to make it more readable
Codecov Report
@@ Coverage Diff @@
## master #108 +/- ##
==========================================
+ Coverage 41.98% 42.62% +0.64%
==========================================
Files 6 6
Lines 574 563 -11
==========================================
- Hits 241 240 -1
+ Misses 313 303 -10
Partials 20 20
Continue to review full report at Codecov.
|
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.
LGTM!👌
@DHaussermann Please do a smoke test on the changes |
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.
Tested and passed
- No Build errors
- Smoke test admin side config and granting access to non-sysadmin users
- Tested various patterns in posts
No issues found
LGTM!
Summary
This PR copies most of the golangci-lint configuration from mattermost/mattermost-plugin-starter-template#90 into this repo. This helps with a consistent code style and should improve code quality over time.
Most changes are straightforward and just style wise. I did comment on the more tricky ones.
This is an example PR on how to integrate GolangCI-Lint. It should be used as an example for other plugins also adopting the tool.