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

Switch workerd/util to idiomatic comment style #901

Merged
merged 1 commit into from
Aug 18, 2023

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jul 21, 2023

Wanted to get a sense of how difficult and time consuming it would be to manually convert the comment style to the more idiomatic comments-before-declarations style. Turns out it's rather quick and generally painless. This batch took about 30 minutes with several interruptions. Most of it is purely mechanical. In a few cases it's obvious that a tool would be difficult since some comments are obviously impl details that do, for instance, belong inside a function body, while others are clearly meant to describe the declaration. Some judgement is required to pick those out case by case but it seems to be rather painless overall.

@jasnell jasnell changed the title Switch workerd/util to idiomatic comment style [Experimental] Switch workerd/util to idiomatic comment style Jul 21, 2023
Copy link
Collaborator

@harrishancock harrishancock left a comment

Choose a reason for hiding this comment

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

LGTM! I think we should do it, one file at a time.

@jasnell jasnell force-pushed the jsnell/comment-reformat-part-1 branch from 5bc746f to 4bda277 Compare August 18, 2023 18:02
@jasnell jasnell marked this pull request as ready for review August 18, 2023 18:02
@jasnell jasnell changed the title [Experimental] Switch workerd/util to idiomatic comment style Switch workerd/util to idiomatic comment style Aug 18, 2023
@jasnell jasnell merged commit 53f0da1 into main Aug 18, 2023
@jasnell jasnell deleted the jsnell/comment-reformat-part-1 branch August 18, 2023 19:02
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