[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

Add ESlint and Conform Apps & Packages #589

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

Add ESlint and Conform Apps & Packages #589

wants to merge 27 commits into from

Conversation

versecafe
Copy link
Contributor

PR to match proposal #581, adds ESlint rules matching eslint:recommended turborepo guidelines, prettier formatting, typescript-eslint, and relevant vercel style guides.

@vercel Vercel
Copy link
vercel bot commented Jan 29, 2024

@versecafe is attempting to deploy a commit to the Elegance Team on Vercel.

A member of the Team first needs to authorize it.

@versecafe versecafe marked this pull request as draft January 29, 2024 19:05
@socket-security Socket Security
Copy link
socket-security bot commented Jan 29, 2024 •
edited
Loading

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@typescript-eslint/eslint-plugin@6.11.0 Transitive: environment, eval, filesystem, shell, unsafe +113 55.2 MB jameshenry
npm/@typescript-eslint/parser@6.11.0 Transitive: environment, eval, filesystem, shell, unsafe +108 52.3 MB jameshenry
npm/@vercel/style-guide@5.1.0 Transitive: environment, eval, filesystem, shell, unsafe +362 88 MB vercel-release-bot
npm/eslint-config-next@14.0.5-canary.55 unsafe Transitive: environment, eval, filesystem, shell +246 68.6 MB vercel-release-bot
npm/eslint-plugin-only-warn@1.1.0 None 0 6 kB bfanger

🚮 Removed packages: npm/@radix-ui/colors@0.1.8, npm/@radix-ui/react-popover@1.0.2, npm/@radix-ui/react-toggle-group@1.0.1, npm/@radix-ui/react-tooltip@1.0.2, npm/@types/classnames@2.3.1, npm/@types/react-dom@18.0.9, npm/@types/react@18.0.25, npm/autoprefixer@10.4.13, npm/eslint-config-next@13.2.4, npm/eslint-config-prettier@8.7.0, npm/eslint-plugin-simple-import-sort@10.0.0, npm/eslint-plugin-unused-imports@2.0.0, npm/eslint@8.36.0, npm/next@14.0.5-canary.55, npm/postcss@8.4.19, npm/prism-react-renderer@1.3.5, npm/tailwindcss@3.2.4, npm/typescript@4.9.3

View full report↗︎

@versecafe
Copy link
Contributor Author

@steven-tey when setting up turbo.json files for the packages I noticed that some env vars are shared between packages but there is no top level env file, would it make sense to move things like

"NEXT_PUBLIC_APP_SHORT_DOMAIN",
"NEXT_PUBLIC_APP_DOMAIN",
"NEXT_PUBLIC_APP_NAME",
"NEXT_PUBLIC_VERCEL_ENV",
"NEXT_PUBLIC_IS_DUB",

up to a top level env file and declare them in the root turbo.json

@versecafe versecafe marked this pull request as ready for review February 5, 2024 03:36
@versecafe
Copy link
Contributor Author

@steven-tey This should now be ready for review, the rules are built around vercel guidelines, turborepo eslint, and recommended next lint rules

there are approximately 20-30 warnings in web and utiles and around 60 in ui they are mostly stemming from default export rules and other small things such as regex, I didn't want to touch them for now due to the risk of merge conflicts when changing hundreds of functions to be named and modifying their import method. total warnings are down from a baseline of around 600 when the rules were first implemented.

@versecafe
Copy link
Contributor Author

Conformance

utils: ✖ 24 problems (0 errors, 24 warnings)
ui: ✖ 57 problems (0 errors, 57 warnings)

web: ✖ 27 problems (0 errors, 27 warnings)

@CLAassistant
Copy link
CLAassistant commented Mar 12, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@versecafe @CLAassistant @steven-tey