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

feat: small improvements #256

Merged
merged 7 commits into from
Feb 9, 2025

Conversation

kaffolder7
Copy link
Collaborator

This PR includes a few small, but I think helpful changes. ☺️ Each commit has notes.

kaffolder7 and others added 6 commits February 5, 2025 18:40
Switching from Google Fonts to Bunny Fonts CDN. Bunny Fonts is an open-source, privacy-first web font platform. It is fully GDPR compliant (Google is not) and can act as a drop-in replacement for Google Fonts.
Default documentation link in main nav won’t work because /docs are not included in built Docker images (to keep image smaller). Instead, changing seed data to point to the docs homepage. :)
root = true

# Default settings for all files
[*]
Copy link
Owner

Choose a reason for hiding this comment

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

can we keep it similar to .prettierrc for tab size and tabs

{
	"useTabs": true,
	"semi": true,
	"tabWidth": 4,
	"trailingComma": "none",
	"printWidth": 100,
	"plugins": ["prettier-plugin-svelte", "prettier-plugin-tailwindcss"],
	"overrides": [{ "files": "*.svelte", "options": { "parser": "svelte" } }]
}

Copy link
Owner

Choose a reason for hiding this comment

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

so indent_size=4 and indent with tabs instead of spaces

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that is fine. I only specified two spaces instead of 4 spaces/tab because two spaces are more of the standard when it comes to modern JavaScript and Svelte. I had checked with GPT. However, I know everyone has their own preference, and because you are the code owner I will get this changed. Just wanted to make a best-practice/standardized suggestion, 😉 ...but I have no desire to raise argument over this. 😂 If you don't think this standard is a good change, I think the real win here is implementing an EditorConfig file to help others more quickly align to project standards in their own IDEs.

I can get this change made to reflect your settings in Prettier if that's ultimately what you would like. Are you expecting only .js, .tx, and Svelte file types updated to tabs?

Copy link
Collaborator Author

@kaffolder7 kaffolder7 Feb 8, 2025

Choose a reason for hiding this comment

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

@rajnandan1 Here's what I'm proposing for best-practice and standardized approach. I can easily update .prettierrc to match:

Filetype Indent Size Indent Style Notes
* 4 tab Catch-all for files not overridden in other rules below
*.svelte 4 tab rajnandan1's preference

Standard/best-practice and more-widely adopted is actually: 2 spaces
*.{js,ts,tsx,cjs,mjs} 4 tab rajnandan1's preference

Standard/best-practice and more-widely adopted is actually: 2 spaces
*.json,.prettierrc 2 spaces per JSON (RFC 8259) specification
*.{yaml,yml} 2 spaces per latest YAML 1.2 (2009) specification
*.md 4 tab rajnandan1's preference

Standard/best-practice and more-widely adopted is actually: 2 or 4 spaces
Dockerfile 4 tab -

Could you please confirm if this is ok? Or if you would like any of these adjusted further? The "Indent Size" and "Indent Style" columns are currently the values I will plan on implementing, unless you are comfortable with my suggestions in the "Notes" column. 😊

Copy link
Owner

@rajnandan1 rajnandan1 Feb 8, 2025

Choose a reason for hiding this comment

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

I feel you are correct and we can keep it 2 in .editorconfig which should be fine.

Copy link
Owner

Choose a reason for hiding this comment

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

Merge? if there is nothing else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rajnandan1 Yes, you may merge. I just pushed the updates. .prettierrc and .editorconfig are now better-aligned w/ best-practice/most-widely adopted indentation sizes and formats. 😊

Aligned `.prettierrc` and `.editorconfig` files to same, best-practice & most-widely-adopted standards.
@rajnandan1 rajnandan1 merged commit 1147808 into rajnandan1:main Feb 9, 2025
@kaffolder7 kaffolder7 deleted the feature/small-improvements branch February 9, 2025 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants