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

Coding style: add a section on list ordering. #505

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

jrvanwhy
Copy link
Collaborator

I've been keeping a lot of things (such as Cargo.toml package lists and mod declarations) in alphabetical order. This adds that preference to the style guide.

Rendered

I've been keeping a lot of things (such as `Cargo.toml` package lists and `mod` declarations) in alphabetical order. This adds that preference to the style guide, so other contributors are aware of it.
@jrvanwhy jrvanwhy added the upkeep Indicates a PR is upkeep as defined by the code review policy. label Aug 22, 2023
@bradjc
Copy link
Contributor

bradjc commented Aug 22, 2023

I'm pretty opposed to this, but I won't stand in the way. If this kind of thing isn't enforced by a tool ala rustfmt, I don't see how this is a good use of developer effort. Just my 2¢.

I do think it is reasonable to say this is the expected style but not enforce it on each PR, and then just open PRs like this one from time to time. I do that with a few things in tockland.

Also, maybe this is coming? rust-lang/rustfmt#5240 I didn't read too closely.

Copy link
Member

@lschuermann lschuermann left a comment

Choose a reason for hiding this comment

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

Whenever I walk across a large list such as workspace and it's not sorted it makes me sad.

@jrvanwhy
Copy link
Collaborator Author

I do think it is reasonable to say this is the expected style but not enforce it on each PR, and then just open PRs like this one from time to time. I do that with a few things in tockland.

I'm okay with this plan, since we don't have a way to enforce it automatically.

@jrvanwhy
Copy link
Collaborator Author

I will merge as-is, but we won't proactively enforce this during PR reviews, unless doing so would avoid a merge conflict.

@jrvanwhy jrvanwhy added this pull request to the merge queue Aug 23, 2023
Merged via the queue into tock:master with commit 571e1c2 Aug 23, 2023
3 checks passed
@jrvanwhy jrvanwhy deleted the style-list-order branch August 23, 2023 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upkeep Indicates a PR is upkeep as defined by the code review policy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants