Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-22155 ESLint configuration updates part 1 #5544

Merged
merged 7 commits into from
May 25, 2020
Merged

MM-22155 ESLint configuration updates part 1 #5544

merged 7 commits into from
May 25, 2020

Conversation

hmhealey
Copy link
Member

@hmhealey hmhealey commented May 20, 2020

This is the first part of the configuration changes proposed here. It's focused on the simpler things to change like allowing undefined since it's better supported in TypeScript, getting rid of a few annoying warnings (file extensions and magic numbers), and re-enabling some stuff that was disabled during the TypeScript migration (camelcase naming, nested ternaries, and unused variables).

The biggest change in this one is that re-enabling the no-unused-vars check requires a bunch of redux action creators to have their return type changed, although this is consistent with what we had in the past.

Most of the rules changes in the .eslintrc.json will be moved to our ESLint plugin later once all the repos are consistent.

Ticket Link

https://mattermost.atlassian.net/browse/MM-22155

Related Pull Requests

mattermost/mattermost-mobile#4311
mattermost/mattermost-redux#1158

Copy link
Member

@devinbinnie devinbinnie left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple questions.

utils/markdown/renderer.tsx Show resolved Hide resolved
@@ -68,7 +68,7 @@ describe('Paste.formatGithubCodePaste', () => {
const message = "```\n// a javascript codeblock example\nif (1 > 0) {\n return 'condition is true';\n}\n```";
const codeBlock = "```\n// a javascript codeblock example\nif (1 > 0) {\n return 'condition is true';\n}\n```";

const {formattedMessage, formattedCodeBlock} = formatGithubCodePaste(0, '', clipboardData);
const {formattedMessage, formattedCodeBlock} = formatGithubCodePaste(0, '', clipboardData); // eslint-disable-line @typescript-eslint/no-unused-vars
Copy link
Member

Choose a reason for hiding this comment

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

Same with these.

Copy link
Member Author

Choose a reason for hiding this comment

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

I filed a ticket to fix these. The tests weren't written correctly, so I added the ignore comments so that the argument handling doesn't need to be re-added when the tests are fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Once #5559 is merged, these can be removed. I'll do that in this PR if that one makes it in first.

@hmhealey hmhealey added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels May 25, 2020
@hmhealey
Copy link
Member Author

Skipping dev review since these are only code style changes

@hmhealey hmhealey merged commit 4edeadc into master May 25, 2020
@hmhealey hmhealey deleted the MM-22155 branch May 25, 2020 19:03
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jul 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation
Projects
None yet
4 participants