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

Add editorconfig #821

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Conversation

NeilGirdhar
Copy link
Contributor

No description provided.

Copy link
Owner

@patrick-kidger patrick-kidger left a comment

Choose a reason for hiding this comment

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

Do you feel that this adds much over the existing autoformatting (via pre-commit + ruff)? It feels more natural to me to have just a single place that handle this.

.editorconfig Outdated
Comment on lines 13 to 19
[*.toml]
max_line_length = 150
indent_size = 4

[*.yml]
max_line_length = 120
indent_size = 2
Copy link
Owner

Choose a reason for hiding this comment

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

I'd cut these two. Right now we rarely use either kind of file and both are unformatted.

Copy link
Contributor Author

@NeilGirdhar NeilGirdhar Aug 29, 2024

Choose a reason for hiding this comment

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

This is for editors, not formatters. The indent size tells the editor how many spaces to add when "tab" is pressed. I got rid of the line length then.

@NeilGirdhar
Copy link
Contributor Author

NeilGirdhar commented Aug 29, 2024

editorconfig automatically configures editors like vim, emacs, etc. so that they know how to save files, how long the lines should be (for examples, vim's automatic line width formatter), where to show the line length helper.

The problem is that I have my editor configured differently than yours. With this file, my editor automatically adjusts to your configuration.

@patrick-kidger
Copy link
Owner

Indeed, I'm familiar with .editorconfig.

My point is that if we care about such things then IMO it'd be better to have a single source of truth on such matters -- an autoformatter -- and allow anything at all whilst editing.

Anyway, not a strong opinion. *.py files are the only ones we attempt to format so trim this down to just that and I'd be happy to merge this.

@NeilGirdhar NeilGirdhar force-pushed the editorconfig branch 2 times, most recently from ea73e65 to 03309ae Compare August 30, 2024 08:10
@NeilGirdhar
Copy link
Contributor Author

NeilGirdhar commented Aug 30, 2024

My point is that if we care about such things then IMO it'd be better to have a single source of truth on such matters -- an autoformatter -- and allow anything at all whilst editing.

Yes, I see your point. I guess I forgot that you've got the auto-formatter enabled.

Anyway, updated as requested.

@patrick-kidger patrick-kidger merged commit 2508192 into patrick-kidger:main Aug 30, 2024
2 checks passed
@patrick-kidger
Copy link
Owner

Alright, merged! :) Thank you for the contribution!

@NeilGirdhar
Copy link
Contributor Author

NeilGirdhar commented Aug 30, 2024

Thanks for being flexible about this :). Maybe we should propose to Ruff that editorconfig be the defaults to satisfy your "one source of truth" desire?

@NeilGirdhar NeilGirdhar deleted the editorconfig branch August 30, 2024 11:41
@patrick-kidger
Copy link
Owner

Seems not planned: astral-sh/ruff#1530

@NeilGirdhar
Copy link
Contributor Author

Ah, nice find!

@patrick-kidger patrick-kidger mentioned this pull request Sep 14, 2024
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