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

Formatted project using cargo fmt #28

Closed
wants to merge 1 commit into from

Conversation

skytz
Copy link

@skytz skytz commented Jun 13, 2023

No description provided.

@kurtbuilds
Copy link
Owner

kurtbuilds commented Jun 17, 2023

I appreciate this contribution, but I'm going to decline it.

Today, I code in IntelliJ and use its formatter. I'm open to rustfmt, so after getting your PR, I tried playing with it for ormlite & other projects.

All of the following is personal opinion: Right now, rustfmt configuration & features are in a state where I find it quite frustrating to use. The defaults are way too aggressive with indentation and line wrapping. Even with effort, I wasn't able to find configuration options to resolve all my pain points. Moreover, it makes a number of non-configurable choices that I think are unquestionably worse. I opened an issue about one in particular where's rustfmt causes almost identical code to format fairly differently (rust-lang/rustfmt#5783) and in that interaction the team seemed unwilling to engage in the substance of the issue I filed.

Often, rustfmt does a good job of formatting, but for now, I think manual formatting is still superior, and there aren't so many contributors where formatting inconsistency is impacting the project.

I'm open to smaller scoped PRs, PRs where rustfmt is only run on code you're already touching, or the addition of a .rustfmt.toml file with:

ignore = ["/"]

if you have default editor settings that otherwise prevent you from contributing to the project. (Note that ignore is still unstable.)

@kurtbuilds kurtbuilds closed this Jun 17, 2023
@skytz
Copy link
Author

skytz commented Jun 24, 2023

I understand your point, and i will format the project manually on the code i wrote.

My take on the formatting issue is simple:
Consistent formatting (even if it looks bad in some cases) is better than manually formatting, especially when working in a team (or in this case open source).
Mostly because i can check if the formatting is the same across team members.
Combining it with clippy, i'm resonably sure everyone's code looks kinda the same.

If your issue is that in some cases, like the example you gave in the rustfmt issue, rustfmt doesn't do what you want, #[rustfmt::skip] should fix the problem.
If its a general thing ... 🤷‍♂️.

This problem won't dissapear if other people want to contribute to the package, and some formatting rules would help reduce friction and overhead.

@kurtbuilds
Copy link
Owner

Definitely. It's something I'd revisit when the rustfmt gets more configuration/development/love and/or this project has many more contributors to make consistency that much more important.

franklx pushed a commit to franklx/ormlite that referenced this pull request Aug 18, 2023
Remove the unecessary file lock, attempt to rename before copying.
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.

None yet

2 participants