-
Notifications
You must be signed in to change notification settings - Fork 81
ci: reflow comments using awk #1413
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
base: next
Are you sure you want to change the base?
Conversation
2d961c3 to
7c21641
Compare
7c21641 to
692907c
Compare
692907c to
149ed49
Compare
Mirko-von-Leipzig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this actually worked very well.
| // diesel::joinable!(notes -> accounts (sender)); diesel::joinable!(transactions -> accounts | ||
| // (account_id)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one outlier which is "malformatted" but I think likely we should place this in a code fence or something.
I'm leaving this as is for now just for discussion's sake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, should have been a code block in the first place - or deleted.
|
|
||
| // The rebuild is automatically triggered by `build.rs` as described in | ||
| // <https://docs.rs/diesel_migrations/latest/diesel_migrations/macro.embed_migrations.html#automatic-rebuilds>. | ||
| // The rebuild is automatically triggered by `build.rs` as described in <https://docs.rs/diesel_migrations/latest/diesel_migrations/macro.embed_migrations.html#automatic-rebuilds>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is arguably worse as well now, but its because the link exceeds the limit and rustfmt doesn't know what to do. This might behave better if it was a doc comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be great to have an opt out way for it, i.e. like clang-format. But the awk is already scarily complex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we can probably get it to respect some kind of // [rustfmt::skip] pretty easily. But I think in this case its a problem with the < ... >
| @@ -1,5 +1,4 @@ | |||
| // This build.rs is required to trigger the `diesel_migrations::embed_migrations!` proc-macro in | |||
| // `store/src/db/migrations.rs` to include the latest version of the migrations into the binary, see <https://docs.rs/diesel_migrations/latest/diesel_migrations/macro.embed_migrations.html#automatic-rebuilds>. | |||
| // This build.rs is required to trigger the `diesel_migrations::embed_migrations!` proc-macro in `store/src/db/migrations.rs` to include the latest version of the migrations into the binary, see <https://docs.rs/diesel_migrations/latest/diesel_migrations/macro.embed_migrations.html#automatic-rebuilds>. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also a failure point; unclear why except rustfmt thought this was the way to do it 🤷
| } | ||
|
|
||
| next | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
End of poor-mans-markdown parser 😰
| } | ||
|
|
||
| END { if(in_comment) print out_prefix merged } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've such an itch to fixup cargo-spellcheck reflow now...
I'm experimenting with using
awkto reflow comment lines.The awk script is generated via AI (because doing this manually.. !@#% that :)).
Flow is basically:
reflow.shcargo +nightly fmt --allto truncate the overly long comment lines againIt should handle and be considerate of
============...delimitersIn addition I've modified the makefile to include the flow. The fmt checker in CI now runs the flow and ensures that the diff is zero.