Skip to content

Conversation

@sgued
Copy link
Contributor

@sgued sgued commented Oct 30, 2025

This follows the wg meeting conclusions of 28/10/2025

Copy link
Contributor

@zeenix zeenix left a comment

Choose a reason for hiding this comment

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

Great stuff otherwise! 👍

src/mpmc.rs Outdated
/// in a state were no other task can successfully enqueue or dequeue items from it
/// until the pre-empted task can finish its operation.
///
/// In that case, [`enqueue`](QueueInner::dequeue) and [`dequeue`](QueueInner::enqueue) will return an error, but will not panic or reach undefined behaviour
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep the comment lines also under 100 chars please?

It would be great to automate this with comment_width = 100 in the .rustfmt.toml but unfortunately that's still nightly-only. In all my projects I have always used the nightly fmt for this (and 2 other nightly-only options) and we can do so in this project too if you'd be OK with that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the lines that were touched by this PR that are longer than 100 characters.

I think we already talked about it. I don't really like it:

  • it means that running just cargo fmt without nightly will fail
  • I don't think that we should be forced to wrap comments when just one word is going over the 100 character limit
  • my IDE wraps comment properly, keeping indentation and all so I never have an issue with readability
  • It can force us to re-format code examples in a way that doesn't make sense for documentation.

But I'm fine with merging it if you make a PR for this

Copy link
Contributor

Choose a reason for hiding this comment

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

it means that running just cargo fmt without nightly will fail

It won't. It'll just give a warning. It'll only become an error if you explicitly ask for warnings to be treated as errors. We'll do that in the CI but we'll then be running nightly.

As I wrote I've been using this in many projects for many years without much of a problem. I'm talking about projects that get sufficient money of (new) contributions.

It's unfortunate that rustfmt is sort of unmaintained so these essential features remain to be unstable forever. 😓


Anyway, up to you. It's just annoying that I have to point out this inconsistency (lines in the same files should have the same max length) all the time to you. Please manage it manually if you're against automating this. 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It won't. It'll just give a warning. It'll only become an error if you explicitly ask for warnings to be treated as errors

Oh, right.

Then yeah let's go for it, just make a PR setting it up the way you usually do I'll approve it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that we should be forced to wrap comments when just one word is going over the 100 character limit

Yeah, you should be. We want to keep it as consistent as possible. You're against automating this, you have to do this manually then. Sorry!

my IDE wraps comment properly, keeping indentation and all so I never have an issue with readability

That's entirely between you and your IDE.

It can force us to re-format code examples in a way that doesn't make sense for documentation.

It's code and it needs to follow the same rules as the rest of the code (even if the /// takes a few chars away) anyway. I could be misremembering but I think there's also an unstable option for formatting the code inside doc comments. 🤔

We need to agree on a line limit and we all need to follow it to keep things consistent. The whole reason for rustfmt is that we don't need to have such debates. rustfmt has already chosen 100 chars limit for us, let's stick to that and keep things consistent.

@sgued sgued force-pushed the mpmc-issue-documentation branch from 0c69fac to 73bfcf4 Compare November 5, 2025 06:51
@sgued sgued requested a review from zeenix November 5, 2025 07:04
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