-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
chore: overhaul ignore configs #11881
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.
|
||
# 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_modules
entry..env
file rules - only the*.local
files are ignored by git..eslintignore
that are not handled by ESLint, such as.DS_STORE
..eslintignore
, such asdist
which is generated in the library template.Thumbs.db
for Windows users.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits