switch block-style to line-style comments#553
Merged
Merged
Conversation
david-crespo
approved these changes
Jan 9, 2023
ahl
reviewed
Jan 9, 2023
ahl
left a comment
Collaborator
There was a problem hiding this comment.
checked everywhere for a similar problem but just found this one
git log --no-color -p main.. | grep '^-[^*]*$'
Comment on lines
+190
to
+206
| /// #[dropshot::endpoint { method = GET, path = "/my/ws/endpoint/{id}" }] | ||
| /// async fn my_ws_endpoint( | ||
| /// rqctx: std::sync::Arc<dropshot::RequestContext<()>>, | ||
| /// websock: dropshot::WebsocketUpgrade, | ||
| /// id: dropshot::Path<String>, | ||
| /// ) -> dropshot::WebsocketEndpointResult { | ||
| /// let logger = rqctx.log.new(slog::o!()); | ||
| /// websock.handle(move |upgraded| async move { | ||
| /// slog::info!(logger, "Entered handler for ID {}", id.into_inner()); | ||
| /// use futures::stream::StreamExt; | ||
| /// let mut ws_stream = tokio_tungstenite::WebSocketStream::from_raw_socket( | ||
| /// upgraded.into_inner(), tokio_tungstenite::tungstenite::protocol::Role::Server, None | ||
| /// ).await; | ||
| /// slog::info!(logger, "Received from websocket: {:?}", ws_stream.next().await); | ||
| /// Ok(()) | ||
| /// }) | ||
| /// } |
Collaborator
Author
There was a problem hiding this comment.
Great catch! For my future reference, I think it's because it wasn't formatted the way standard block-style comments are formatted (missing leading asterisks on those lines). I fixed this by reverting this file, fixing the C-style formatting, then rerunning cargo +nightly fmt -- --config-path=rustfmt-new.toml.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is modeled after oxidecomputer/omicron#770. Paraphrasing from there what applies here as well.
Dropshot currently has a mix of commenting styles -- both block style (C style, /* ... */) and line style (// ...). It seems like this has been a distraction for folks, and the line style is more popular. After the success of oxidecomputer/omicron#770 I used rustfmt's
normalize_commentsoption to switch everything to line style. I don't think we should enforce this with rustfmt because that requires an unstable option, which we know from experience is painful. I don't expect this to be a big problem because I think line style tends to be the more common expectation.When this lands, it may be annoying to merge with outstanding work. I'm happy to help with that. Based on last time, when this commit lands on "main", I think the right strategy is to sync up other PRs by:
rustfmt-new.toml:cargo +nightly fmt -- --config-path=rustfmt-new.tomlgit commit -am 'premerge with style update'