-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
chore: overhaul ignore configs #11881
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
Conversation
|
|
|
||
| build | ||
| node_modules | ||
| package |
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.
I think it could lead to trouble to remove the leading /. If the user has something like src/packages/package as we do in SvelteKit it would end up getting ignored, which isn't what we want
| .output | ||
| .svelte-kit | ||
| .vercel | ||
|
|
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.
build and .svelte-kit are both output, so should be grouped together.
| .svelte-kit | ||
| .vercel | ||
|
|
||
| # Ignore files for PNPM, NPM and YARN |
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.
are we sure eslint ignores these files? how do we know that?
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.
JSON and YAML files are not formatted by default.
Plugins have to be installed:
- JSON: https://github.com/kuceb/eslint-plugin-json-format
- YAML: https://github.com/ota-meshi/eslint-plugin-yml
And the extensions should be provided in the CLI and configuration.
| /build | ||
| /.svelte-kit | ||
| /package | ||
| .env |
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.
If I recall, the reason this was ignored was that people were worried about accidentally checking in production secrets. It makes sense to me to ignore it because you're not going to check it in here, but will instead provide it as part of your production deploy
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.
Ignoring the .env file goes agaist the Vite convention.
https://vitejs.dev/guide/env-and-mode#env-files
.env # loaded in all cases
.env.local # loaded in all cases, ignored by git
.env.[mode] # only loaded in specified mode
.env.[mode].local # only loaded in specified mode, ignored by git
Since SvelteKit is at it core a Vite project, following this seemed intuitive.
Especially since .env.test and .env.test.local files are utilized by Vitest.
|
@benmccann I am really sorry, I badly messed up my git branching. Should I create a new PR with the above mentioned suggestions? |
|
Go for it @hyunbinseo |
Update git, Prettier, and ESLint ignore files.
/- match the style with thenode_modulesentry..envfile rules - only the*.localfiles are ignored by git..eslintignorethat are not handled by ESLint, such as.DS_STORE..eslintignore, such asdistwhich is generated in the library template.Thumbs.dbfor Windows users.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits