Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Conversation

@kianenigma
Copy link
Contributor

Closes #6222

This makes the code much more readable for anyone who has visual rules in their editor, and just much more appealing over all.

I have to go through them once and revert maybe the links and bash commands in comments.

@kianenigma kianenigma added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Aug 3, 2021
@kianenigma kianenigma requested review from bkchr and expenses August 3, 2021 15:00
@athei
Copy link
Member

athei commented Aug 4, 2021

I did not review all files (obviously) but from what I have seen it does improve the visual appearance.

Copy link
Contributor

@gilescope gilescope left a comment

Choose a reason for hiding this comment

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

I'm generally in favor of us turning this on. I think it has more merit than the reformatting strings flag and on the whole looks to do more good than harm but there are definitely (highlighted) places where we need to opt out of the auto-formatting somehow.

@kianenigma
Copy link
Contributor Author

@gilescope good catch. The trick is simply wrap anything in ", ', or ` and it will not automatically wrap, which seems like a correct thing to do anyway.

@kianenigma
Copy link
Contributor Author

@gilescope I belive I have addressed all your issues, except a handful of ones that I didn't resolve.

I am still strongly for this change. Most of the things that we had to fix manually are either code snippets, which can be #[rustfmt::skipp]ed, or due to using non-standard markdown.

@kianenigma kianenigma requested a review from gilescope August 5, 2021 14:49
@kianenigma kianenigma force-pushed the kiz-format-comments branch from 24f62b6 to b5cc0e4 Compare August 10, 2021 14:01
Copy link
Contributor

@expenses expenses left a comment

Choose a reason for hiding this comment

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

99% of these changes look great :D ! But the remaining 1% do not. I think you'll need to manually change a few comments and then re-format.

@kianenigma kianenigma requested a review from expenses August 10, 2021 15:06
Copy link
Contributor

@expenses expenses left a comment

Choose a reason for hiding this comment

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

I don't really to review the whole thing again, but I think that if you ran cargo +nightly fmt again it should be good to go.

@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Aug 11, 2021

Waiting for commit status.

@ghost
Copy link

ghost commented Aug 11, 2021

Merge aborted: Head SHA changed from 7938cac to b3765f6

@kianenigma
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Aug 11, 2021

Waiting for commit status.

@ghost ghost merged commit 4c3a55e into master Aug 11, 2021
@ghost ghost deleted the kiz-format-comments branch August 11, 2021 14:56
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify doc/comment line width.

8 participants