rust-cli / team

CLI working group
https://rust-cli.github.io/book
MIT License
294 stars 34 forks source link

Fix code style #144

Closed JohnTitor closed 4 years ago

JohnTitor commented 4 years ago

Run clippy and rustfmt

stappersg commented 4 years ago

LGTM

killercup commented 4 years ago

Please note that the original formatting was chosen so that the code looks good when included in the book! Is that still the case?

stappersg commented 4 years ago

Looks good to me, the long version:

Patch

It is not important why this patch needed to wait seven weeks before it got it's first response. Important is that someone says which parts of this patch are good for acceptence.

killercup commented 4 years ago

@stappersg, thank you for the review. Sadly, the main issue is that this code is also included in the rendered book, where a different style might be required. The reason why it takes long for people to review PRs is that this is a project run by volunteers who only have so many hours per day 🤷‍♂

spacekookie commented 4 years ago

It is not important why this patch needed to wait seven weeks before it got it's first response. Important is that someone says which parts of this patch are good for acceptence.

Not sure it's exactly nice to shame people maintaining public resources for not responding fast enough.

Apart from that, I feel that sorting imports is probably a trivial change, but changing the formatting of code sections is not something that I'd want to have done on a global scale. The point of code examples isn't to be idiomatic, but to be easy teaching resources. As such, formatting might not be the most important aspect of it

JohnTitor commented 4 years ago

Hm, I have no strong opinion for this, just close then.

stappersg commented 4 years ago

Hm, I have no strong opinion for this, just close then.

Thanks for taking action.

Please, pretty please, keep submitting merge requests.