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

Alphabetize language features #90935

Merged
merged 2 commits into from
Nov 17, 2021
Merged

Conversation

jhpratt
Copy link
Member

@jhpratt jhpratt commented Nov 16, 2021

This should significantly reduce the frequency of merge conflicts.

r? @joshtriplett

@rustbot label: +A-contributor-roadblock +S-waiting-on-review

@rustbot rustbot added A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2021
@joshtriplett
Copy link
Member

LGTM; r=me when it passes CI. And thanks!

@jhpratt
Copy link
Member Author

jhpratt commented Nov 16, 2021

I'm unable to do r=you 😄

@Mark-Simulacrum
Copy link
Member

Hm. So while this seems OK, it also does make it more annoying to add new features in -- particularly when the tidy lint doesn't tell you where you should put it.

I think there was some conversation about a merge driver potentially letting us do this nicely without needing to change the alphabetization?

@jhpratt
Copy link
Member Author

jhpratt commented Nov 16, 2021

A merge driver could work, but it appeared that neither GitHub nor bors support custom drivers.

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Nov 16, 2021

I would expect that bors should support a .gitattributes specified merge=union for this file, which I think is correct in most cases -- when no new release has happened?

bors currently just shells out to git, so if we can get that to work, I would prefer it over a solution that makes life harder for those adding to this file (which prevents merge conflicts but doesn't fully remove the pain).

It also seems like it should be possible to define a filter via .gitattributes that sorts the file automatically on git add, perhaps? I haven't read the documentation closely, but it seems like that pretty closely maps to our use case here? (And this would run on developer machines, and so not need bors/github to support it, if I understand correctly).

@jhpratt
Copy link
Member Author

jhpratt commented Nov 16, 2021

Per a discussion on Zulip, I've added a suggestion as to where a feature should be placed. This works on the assumption that features are added at the end of the list, which is currently always the case. If it's added in a different location, the suggestion will technically be correct but is less than ideal.

@Mark-Simulacrum
Copy link
Member

Please add the comment to the end of the list indicating the alphabetical ordering (as suggested by @joshtriplett over Zulip, earlier). I think once that's in place, we can merge.

This should significantly reduce the frequency of merge conflicts.
@jhpratt
Copy link
Member Author

jhpratt commented Nov 16, 2021

Added.

@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 16, 2021

📌 Commit 2a82d1c has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 16, 2021
…htriplett

Alphabetize language features

This should significantly reduce the frequency of merge conflicts.

r? `@joshtriplett`

`@rustbot` label: +A-contributor-roadblock +S-waiting-on-review
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 16, 2021
…htriplett

Alphabetize language features

This should significantly reduce the frequency of merge conflicts.

r? ``@joshtriplett``

``@rustbot`` label: +A-contributor-roadblock +S-waiting-on-review
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 16, 2021
…htriplett

Alphabetize language features

This should significantly reduce the frequency of merge conflicts.

r? ```@joshtriplett```

```@rustbot``` label: +A-contributor-roadblock +S-waiting-on-review
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2021
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90733 (Build musl dist artifacts with debuginfo enabled)
 - rust-lang#90787 (Add `#[inline]`s to `SortedIndexMultiMap`)
 - rust-lang#90920 (:arrow_up: rust-analyzer)
 - rust-lang#90933 (Fix await suggestion on non-future type)
 - rust-lang#90935 (Alphabetize language features)
 - rust-lang#90949 (update miri)
 - rust-lang#90958 (Mark `<*const _>::align_offset` and `<*mut _>::align_offset` as `const fn`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3f55007 into rust-lang:master Nov 17, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 17, 2021
@jhpratt jhpratt deleted the alphabetize-features branch November 17, 2021 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants