Skip to content
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

[GH-232]: Fix e2e playwright directory linting #241

Merged
merged 3 commits into from
Jan 24, 2024
Merged

[GH-232]: Fix e2e playwright directory linting #241

merged 3 commits into from
Jan 24, 2024

Conversation

c0d33ngr
Copy link
Contributor

Summary

This pull request fix linting errors in the e2e/playwright directory.

Ticket Link

Fixes #232

@c0d33ngr c0d33ngr requested a review from mickmister as a code owner January 22, 2024 01:53
@mickmister
Copy link
Contributor

@c0d33ngr Thanks for this contribution 👍

Heads up that we will also want to uncomment the following lines to make the linting run in the Playwright test's CI:

https://github.com/mattermost/mattermost-plugin-todo/blob/594ec224abfc92d9e5c3a646ffe6b212ff25c6db/.github/workflows/playwright.yml#L173-L176


@raghavaggarwal2308 Looks like there is remaining goconst issues in this repo. Could you or another developer please look into resolving the duplicate string issue? Thank you!

Running golangci-lint
golangci-lint run ./...
Error: server/plugin.go:223:18: string `username not valid, err=` has 2 occurrences, make it a constant (goconst)
		p.API.LogError("username not valid, err=" + appErr.Error())
		               ^
Error: server/plugin.go:206:19: string `Unable to add issue err=` has 2 occurrences, make it a constant (goconst)
			p.API.LogError("Unable to add issue err=" + err.Error())
			               ^
Error: server/plugin.go:339:18: string `Unable to write json response err=` has 2 occurrences, make it a constant (goconst)
		p.API.LogError("Unable to write json response err=" + err.Error())
		               ^
make: *** [Makefile:52: check-style] Error 1
Error: Process completed with exit code 2.

@c0d33ngr
Copy link
Contributor Author

I've remove the comments @mickmister 😊

@c0d33ngr
Copy link
Contributor Author

Hi @mickmister , I'd love to resolve the golangci-lint error. Should I create a separate PR for it or resolved it in this PR?

@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (17e4369) 6.42% compared to head (5dcc12e) 6.42%.

❗ Current head 5dcc12e differs from pull request most recent head 50dea37. Consider uploading reports for the commit 50dea37 to get more accurate results

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #241   +/-   ##
======================================
  Coverage    6.42%   6.42%           
======================================
  Files          11      11           
  Lines        1712    1712           
======================================
  Hits          110     110           
  Misses       1594    1594           
  Partials        8       8           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mickmister
Copy link
Contributor

@c0d33ngr Playwright linting is passing now. Nice job 👍

Hi @mickmister , I'd love to resolve the golangci-lint error. Should I create a separate PR for it or resolved it in this PR?

Awesome!! I think a separate PR makes sense since this PR currently has no server-side Go changes. LMK if you have any questions about the task. Thanks @c0d33ngr!

@raghavaggarwal2308
Copy link
Contributor

raghavaggarwal2308 commented Jan 23, 2024

@mickmister @c0d33ngr I have created a PR #242 with the lint error fixes.

Hi @mickmister , I'd love to resolve the golangci-lint error. Should I create a separate PR for it or resolved it in this PR?

@c0d33ngr Apologies from my side, I missed this message.

@c0d33ngr
Copy link
Contributor Author

It's okay @raghavaggarwal2308 😊

@mickmister
Copy link
Contributor

/update-branch

1 similar comment
@mickmister
Copy link
Contributor

/update-branch

@mickmister
Copy link
Contributor

@c0d33ngr Can you please update your local copy of the master branch, and merge its contents into your branch? Thank you!

@c0d33ngr
Copy link
Contributor Author

I've done it @mickmister

Copy link
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @c0d33ngr!

@mickmister mickmister merged commit 3fb8aab into mattermost-community:master Jan 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix e2e playwright directory linting
4 participants