-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Remove unneeded INITIAL_IDS const #83809
Conversation
r? @ollie27 (rust-highfive has picked a reviewer for you, use r? to override) |
r? @camelid |
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 looks great! The only issue is that I think you forgot to add two IDs.
map.insert("associated-const".to_owned(), 1); | ||
map.insert("required-methods".to_owned(), 1); | ||
map.insert("provided-methods".to_owned(), 1); | ||
map.insert("implementors".to_owned(), 1); | ||
map |
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 you forgot to add methods
and synthetic-implementors
to this list.
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.
There is no methods
ID, only a class. Adding synthetic-implementors
.
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.
Ah, okay 👍
7073140
to
13e482b
Compare
Thanks! @bors r+ rollup |
📌 Commit 13e482b has been approved by |
…r=camelid Remove unneeded INITIAL_IDS const Some IDs inside this map didn't exist anymore, some others were duplicates of what we have inside `IdMap`. So instead of keeping the two around and since `INITIAL_IDS` was only used by `IdMap`, no need to keep both of them.
…r=camelid Remove unneeded INITIAL_IDS const Some IDs inside this map didn't exist anymore, some others were duplicates of what we have inside `IdMap`. So instead of keeping the two around and since `INITIAL_IDS` was only used by `IdMap`, no need to keep both of them.
…r=camelid Remove unneeded INITIAL_IDS const Some IDs inside this map didn't exist anymore, some others were duplicates of what we have inside `IdMap`. So instead of keeping the two around and since `INITIAL_IDS` was only used by `IdMap`, no need to keep both of them.
…r=camelid Remove unneeded INITIAL_IDS const Some IDs inside this map didn't exist anymore, some others were duplicates of what we have inside `IdMap`. So instead of keeping the two around and since `INITIAL_IDS` was only used by `IdMap`, no need to keep both of them.
Rollup of 8 pull requests Successful merges: - rust-lang#73945 (Add an unstable --json=unused-externs flag to print unused externs) - rust-lang#81619 (Implement `SourceIterator` and `InPlaceIterable` for `ResultShunt`) - rust-lang#82726 (BTree: move blocks around in node.rs) - rust-lang#83521 (2229: Fix diagnostic issue when using FakeReads in closures) - rust-lang#83532 (Fix compiletest on FreeBSD) - rust-lang#83793 (rustdoc: highlight macros more efficiently) - rust-lang#83809 (Remove unneeded INITIAL_IDS const) - rust-lang#83827 (cleanup leak after test to make miri happy) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Some IDs inside this map didn't exist anymore, some others were duplicates of what we have inside
IdMap
. So instead of keeping the two around and sinceINITIAL_IDS
was only used byIdMap
, no need to keep both of them.