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

Set ESlint rules more strict 🚑 #1000

Closed
wants to merge 42 commits into from
Closed

Set ESlint rules more strict 🚑 #1000

wants to merge 42 commits into from

Conversation

theinterned
Copy link
Member

Issue: -should create one-

This is a continuation of the work by @ndelangen in PR #911

This is in progress

What I did

As described in review comments #904 linting is rather non-strict right now.
Though I'm personally of the opinion too strict linting can be harmful, I agree, we could make it a bit more strict to weed out some bad code we have and prevent bad come from getting pushed to master.

How to test

run npm run lint

ndelangen and others added 27 commits April 17, 2017 16:37
# Conflicts:
#	packages/addon-graphql/package.json
#	packages/addon-info/package.json
#	packages/addon-info/src/components/Node.js
#	packages/addon-knobs/package.json
#	packages/addon-notes/package.json
#	packages/channel-postmessage/package.json
#	packages/channel-websocket/package.json
#	packages/decorator-centered/package.json
#	packages/react-native-storybook/package.json
#	packages/react-storybook/src/server/config.js
#	packages/storybook-ui/package.json
#	packages/storybook-ui/src/modules/ui/components/layout/index.js
notes:

1. did not resolve `jsx-a11y` errors as that would require closer
attention to the browser
2. did not resolve any `'enzyme' should be listed in the project's
dependencies. Run 'npm i -S enzyme' to add it
import/no-extraneous-dependencies` see note:
#911 (comment)
adds `global` as the  dep was missing in add-on-knobs
didn’t resolve jsx-a11y linting in this pass
uses `toMatchObject` rather than `toEqual` to be more lenient about
what props are present
…`import/no-extraneous-dependencies` linter warning
…t` to resolve `import/no-extraneous-dependencies` lint issue
note: didn’t tackle jsx-a11y on this pass.
of airbnb/javascript@121a95d

UPDATED dependencies
CHANGED devDependencies linting
ADD enzyme and storybook-addon to devDependencies in a few packages
@ndelangen
Copy link
Member

✖ 67 problems (57 errors, 10 warnings)

# Conflicts:
#	addons/centered/package.json
#	addons/graphql/package.json
#	addons/info/package.json
#	addons/knobs/package.json
#	addons/links/.storybook/config.js
#	addons/notes/package.json
#	addons/notes/src/index.js
#	addons/notes/src/register.js
#	addons/options/src/manager/index.js
#	addons/storyshots/src/index.js
#	addons/storyshots/stories/required_with_context/Button.stories.js
#	addons/storyshots/stories/required_with_context/Welcome.js
#	app/react-native/package.json
#	app/react-native/src/manager/index.js
#	lib/channel-postmessage/package.json
#	lib/channel-postmessage/src/index.js
#	lib/channel-websocket/package.json
#	lib/channel-websocket/src/index.js
#	lib/cli/generators/METEOR/index.js
#	lib/cli/generators/REACT/template/stories/Welcome.js
#	lib/cli/generators/REACT_SCRIPTS/index.js
#	lib/cli/generators/WEBPACK_REACT/template/stories/Welcome.js
#	packages/react-storybook/src/server/track_usage.js
#	packages/storybook-ui/src/modules/ui/components/layout/usplit.js
#	scripts/prepublish.js
@shilman
Copy link
Member

shilman commented May 30, 2017

@theinterned @ndelangen sorry I missed that this was waiting for my review. We need to get this merged, see e.g. #1163

@theinterned do you mind resolving the merge conflicts and we can get this merged to do a follow-on release? Who knows what other bugs are lurking behind this PR! 😱

@ndelangen
Copy link
Member

closed in favour of #911

@theinterned
Copy link
Member Author

Hi sorry I didn't reply! I was on vacation. Looks like you've got things sorted here though! I'll see if I can help out on the #911 branch

Copy link

nx-cloud bot commented Jul 2, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 08a49db. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants