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

update image version and do cargo fmt #83

Closed
wants to merge 1 commit into from

Conversation

attila-lin
Copy link

No description provided.

@yoanlcq
Copy link
Owner

yoanlcq commented Feb 13, 2022

Hi, this reminds me of #16 , where I explained why I did not want to use rustfmt or cargo fmt for this project.

I am not fundamentally against automated formatting, since it's great and many projects should be using it. However (and as explained in the linked issue), vek has many places which I don't want cargo fmt to touch.

In general also, in this commit, many one-liners were turned into like 4 or 5 lines or more; this reduces the amount of useful code that can fit in one screen.

The only good thing I see is that it sorts the use directives at the top of the modules, and removes some trailing spaces here and there.

I agree that automated formatting is great and that there is some mess that needs cleaning, however I would like it done "right", which requires just a bit too much effort (inserting #[rustfmt_ignore] in many places, tweaking various parameters, and really taking the time to review each file to make sure the formatting is still "good").

Furthermore, since it can be quite subjective, there can be a lot of back-and-forth until I say "looks good to me".

In general though, what I'm looking for is :

  • Code intended to be a one-liner remains a one-liner (this is true for function declarations, function bodies, etc)
  • Code that is specially arranged "visually" (as mentioned in Use rustfmt #16 ) should remain the way it is.

I you can achieve that with cargo fmt then maybe I would consider it; but I wouldn't like to waste your time with that.
I found it simpler to accept the current formatting, imperfect as it is.


About the image update, yes we can do that! If we do, we also need to increment the patch version of vek.

@attila-lin
Copy link
Author

reasonable to me. I will close it.

@attila-lin attila-lin closed this Feb 13, 2022
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