这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@dvulakh
Copy link

@dvulakh dvulakh commented Oct 28, 2025

Switch on formatting for remaining (non-doc) comments.

We conservatively do not format single-line comments whose contents might be code with whitespace-dependent semantics or that may otherwise experience semantic changes upon formatting (namely, comments that contain ", {, or }).

See updated tests.

@dvulakh dvulakh force-pushed the dvulakh.format-comments branch from fa79aea to 502c864 Compare November 3, 2025 20:58
Copy link

@mdelvecchio-jsc mdelvecchio-jsc left a comment

Choose a reason for hiding this comment

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

Review of PR for now, gonna look at offline diff.

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
@dvulakh dvulakh force-pushed the dvulakh.format-comments branch from 502c864 to 2ab7a2b Compare November 3, 2025 21:05
@dvulakh dvulakh changed the base branch from dvulakh.kind-normalization to jane November 3, 2025 21:06
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
added lists for numbers only; letters are likely to be code, I think

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
[1] at start of line specifically

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Copy link

@mdelvecchio-jsc mdelvecchio-jsc left a comment

Choose a reason for hiding this comment

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

I think all of the remaining work is freezing bad comments, the diff looks great though.

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
@mdelvecchio-jsc mdelvecchio-jsc merged commit a2b2800 into jane Nov 7, 2025
2 checks passed
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.

3 participants