Skip to content

Rename ${length()} to ${len()}#124987

Merged
bors merged 2 commits intorust-lang:masterfrom
workingjubilee:macro-metavar-expr-with-a-shorter-len
May 16, 2024
Merged

Rename ${length()} to ${len()}#124987
bors merged 2 commits intorust-lang:masterfrom
workingjubilee:macro-metavar-expr-with-a-shorter-len

Conversation

@workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented May 10, 2024

Implements the rename suggested in #122808 (comment)

I brought this up in the doc PR but it belongs here – length should probably be renamed len before stabilization. The latter is de facto standard in the standard library, whereas the former is only used in a single unstable API. These metafunctions aren’t library items of course, but should presumably still be consistent with established names.

r? @c410-f3r

@rustbot
Copy link
Collaborator

rustbot commented May 10, 2024

Failed to set assignee to c410-f3r: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 10, 2024
@c410-f3r
Copy link
Contributor

c410-f3r commented May 10, 2024 •
edited
Loading

length is the identifier chosen and accepted in the RFC (rust-lang/rfcs#3086).

Any changes should probably occur with the authorization of the responsible team.

cc @markbt

@workingjubilee workingjubilee added T-lang Relevant to the language team I-lang-nominated Nominated for discussion during a lang team meeting. and removed I-lang-nominated Nominated for discussion during a lang team meeting. labels May 10, 2024
@workingjubilee
Copy link
Member Author

workingjubilee commented May 10, 2024

r? lang

@scottmcm scottmcm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. I-lang-nominated Nominated for discussion during a lang team meeting. labels May 13, 2024
@scottmcm
Copy link
Member

scottmcm commented May 13, 2024

Nominating for team discussion. I'm pretty sure this is lang, but I also would be interested in libs-api's opinion for things like this (similar to Err vs Error choices).

@scottmcm scottmcm added S-waiting-on-team and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 13, 2024
@joshtriplett
Copy link
Member

joshtriplett commented May 14, 2024

We discussed this in today's @rust-lang/libs-api meeting. We had a consensus that the standard library consistently uses "len" and not "length", and we recommend that this should be consistent with that as well.

@joshtriplett joshtriplett removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels May 14, 2024
@joshtriplett
Copy link
Member

joshtriplett commented May 15, 2024

We discussed this in today's @rust-lang/lang meeting. We agreed that we should rename this for consistency with usage in the standard library.

Please go ahead with the rename.

@traviscross traviscross removed the I-lang-nominated Nominated for discussion during a lang team meeting. label May 15, 2024
c410-f3r
c410-f3r approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@c410-f3r c410-f3r left a comment

Choose a reason for hiding this comment

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

Code changes LGTM

@workingjubilee
Copy link
Member Author

workingjubilee commented May 16, 2024

hmm, it seems T-libs-api!Josh Triplett and T-lang!Josh Triplett gave it the go-ahead, so

@bors r=c410-f3r,joshtriplett,joshtriplett

that's how it works, right? :^)

@bors
Copy link
Collaborator

bors commented May 16, 2024

📌 Commit 6e74155 has been approved by c410-f3r,joshtriplett,joshtriplett

It is now in the queue for this repository.

@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-team labels May 16, 2024
@bors
Copy link
Collaborator

bors commented May 16, 2024

⌛ Testing commit 6e74155 with merge b71e8cb...

@bors
Copy link
Collaborator

bors commented May 16, 2024

☀️ Test successful - checks-actions
Approved by: c410-f3r,joshtriplett,joshtriplett
Pushing b71e8cb to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 16, 2024
@bors bors merged commit b71e8cb into rust-lang:master May 16, 2024
@rustbot rustbot added this to the 1.80.0 milestone May 16, 2024
@workingjubilee workingjubilee deleted the macro-metavar-expr-with-a-shorter-len branch May 16, 2024 02:47
@workingjubilee workingjubilee mentioned this pull request May 16, 2024
Closed
@bors bors mentioned this pull request May 16, 2024
Merged
@rust-timer
Copy link
Collaborator

rust-timer commented May 16, 2024

Finished benchmarking commit (b71e8cb): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 5.8%, secondary 3.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 5.8% [5.8%, 5.8%] 1
Regressions ❌
(secondary)		 3.8% [3.8%, 3.8%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 5.8% [5.8%, 5.8%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 678.078s -> 679.795s (0.25%)
Artifact size: 316.18 MiB -> 316.18 MiB (-0.00%)

@Veykril Veykril mentioned this pull request May 16, 2024
Closed
@ehuss ehuss mentioned this pull request May 17, 2024
Closed
@NanamiNakano NanamiNakano mentioned this pull request May 26, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@c410-f3r c410-f3r c410-f3r approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@scottmcm scottmcm

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team

Projects

None yet

Milestone

1.80.0

Development

Successfully merging this pull request may close these issues.

8 participants

@workingjubilee @rustbot @c410-f3r @scottmcm @joshtriplett @bors @rust-timer @traviscross