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

Sync reviewers from rust-lang/team #53

Merged
merged 2 commits into from
Jan 1, 2025

Conversation

Noratrieb
Copy link
Member

@Noratrieb Noratrieb commented Apr 8, 2023

Instead of hardcoding everyone, pull in the people that are available there. We then only need to add aliases and additional reviewers, which should decrease the maintenance cost and increase accuracy. As #51 showed, this list here is not very well maintained.

This changes the email addresses for a bunch of people. I needed to add about 40 people to the mailmap (or ensure that they were here) to make it work. In the process, I also mailmapped a few other people that stood out like nrc, who's on thanks twice right now. The updated mailmap is currently committed in this PR, but I will make the change upstream and ping the involved people as a FYI if wanted if this PR is desired. I also had to change the mailmap code to compare names and emails case-insensitively as that caused some problems.

The output changed quite significantly as many clones were merged. I checked the top 100 contributors and after some additional fixes, none of them lost any contributions. It's entirely possible that a few people got split up but I think that's fine for now, as many other are already split up today and get fixed here. We or they can always try to fix it later.

This PR is best reviewed commit by commit, I tried making it as understandable as possible.

fixes #47

@Noratrieb
Copy link
Member Author

I filled out the mailmap according to the following heuristics:
Use the name on rust-lang/team as the proper name.
Use the pre-existing mailmap entry or the email address from the most recent commits from git log as the proper email.

@compiler-errors
Copy link
Member

It would be very cool if we merged this, given that some new reviewers are still left out of this list, and folks don't usually remember that they need to add themselves here <3

@Mark-Simulacrum
Copy link
Member

I don't think I've seen this before. I'm happy to merge it, though it looks like it will need a rebase (and I can review the code but the goal seems good).

@Noratrieb
Copy link
Member Author

I have upstreamed the mailmap changes in rust-lang/rust#132471 and rust-lang/rust#132474

@Noratrieb Noratrieb marked this pull request as draft November 1, 2024 20:03
@Noratrieb
Copy link
Member Author

Noratrieb commented Nov 1, 2024

status: rebased, seems to still work. blocked on the mailmap changes shown above, as we would split a ton of people without those (and who wants to be split?) (and even with those changes, there are gonna be some new splits. i can't address everything, but i at least tried to address many of them).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 1, 2024
…ark-Simulacrum

Add a bunch of mailmap entries

This adds a bunch of missing mailmap entries for many people. These are needed when using rust-lang/team information in rust-lang/thanks (rust-lang/thanks#53), as the emails there may differ.

These are "easy" ones, where there was a mailmap entry already, making it clear which one is the preferred email address.
@GrigorenkoPV
Copy link

This seem to supersede #67, but iirc unicase is unicode-aware, while uncased and git mailmap are not.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 2, 2024
…ark-Simulacrum

Add a bunch of mailmap entries

This adds a bunch of missing mailmap entries for many people. These are needed when using rust-lang/team information in rust-lang/thanks (rust-lang/thanks#53), as the emails there may differ.

These are "easy" ones, where there was a mailmap entry already, making it clear which one is the preferred email address.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 2, 2024
…ark-Simulacrum

Add a bunch of mailmap entries

This adds a bunch of missing mailmap entries for many people. These are needed when using rust-lang/team information in rust-lang/thanks (rust-lang/thanks#53), as the emails there may differ.

These are "easy" ones, where there was a mailmap entry already, making it clear which one is the preferred email address.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 2, 2024
…ark-Simulacrum

Add a bunch of mailmap entries

This adds a bunch of missing mailmap entries for many people. These are needed when using rust-lang/team information in rust-lang/thanks (rust-lang/thanks#53), as the emails there may differ.

These are "easy" ones, where there was a mailmap entry already, making it clear which one is the preferred email address.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Nov 2, 2024
…ark-Simulacrum

Add a bunch of mailmap entries

This adds a bunch of missing mailmap entries for many people. These are needed when using rust-lang/team information in rust-lang/thanks (rust-lang/thanks#53), as the emails there may differ.

These are "easy" ones, where there was a mailmap entry already, making it clear which one is the preferred email address.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 2, 2024
…ark-Simulacrum

Add a bunch of mailmap entries

This adds a bunch of missing mailmap entries for many people. These are needed when using rust-lang/team information in rust-lang/thanks (rust-lang/thanks#53), as the emails there may differ.

These are "easy" ones, where there was a mailmap entry already, making it clear which one is the preferred email address.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 2, 2024
Rollup merge of rust-lang#132471 - Noratrieb:lots-of-mailmapping, r=Mark-Simulacrum

Add a bunch of mailmap entries

This adds a bunch of missing mailmap entries for many people. These are needed when using rust-lang/team information in rust-lang/thanks (rust-lang/thanks#53), as the emails there may differ.

These are "easy" ones, where there was a mailmap entry already, making it clear which one is the preferred email address.
@Noratrieb Noratrieb force-pushed the dyn-reviewers branch 2 times, most recently from 3d57460 to 688f5d8 Compare December 31, 2024 16:23
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Dec 31, 2024
…d-2, r=Mark-Simulacrum

Add more mailmap entries

If you have been pinged: I'm adding a mailmap entry for you to keep thanks.rust-lang.org working properly.

**I have picked the canonical address mostly arbitrarily. If you want a different one as the canonical address, please tell me here**.

<details>
<summary>more details on the change</summary>

builds on top of rust-lang#132471, this containing the less obvious changes that add new canonical email addresses instead of just adding new current ones.

The people updated in this commit have contributed under different email
addresses than the ones they have used in rust-lang/team.
rust-lang/thanks#53 will use team data for thanks reviewers, which requires this to be in
sync.
Therefore, I have updated many of the people that I've noticed being
duplicated after the change.

</details>

I'm adding a novel entry for you: `@apiraino` `@KodrAus` `@cramertj` `@sunfishcode` `@Eh2406` `@skade` `@huonw` `@jsha` `@shepmaster` `@workingjubilee` `@rbtcollins`  `@nrc` `@fitzgen` `@sophiajt` `@tmiasko` `@notriddle` `@TimNN` `@WaffleLapkin`
tgross35 added a commit to tgross35/rust that referenced this pull request Dec 31, 2024
…d-2, r=Mark-Simulacrum

Add more mailmap entries

If you have been pinged: I'm adding a mailmap entry for you to keep thanks.rust-lang.org working properly.

**I have picked the canonical address mostly arbitrarily. If you want a different one as the canonical address, please tell me here**.

<details>
<summary>more details on the change</summary>

builds on top of rust-lang#132471, this containing the less obvious changes that add new canonical email addresses instead of just adding new current ones.

The people updated in this commit have contributed under different email
addresses than the ones they have used in rust-lang/team.
rust-lang/thanks#53 will use team data for thanks reviewers, which requires this to be in
sync.
Therefore, I have updated many of the people that I've noticed being
duplicated after the change.

</details>

I'm adding a novel entry for you: ``@apiraino`` ``@KodrAus`` ``@cramertj`` ``@sunfishcode`` ``@Eh2406`` ``@skade`` ``@huonw`` ``@jsha`` ``@shepmaster`` ``@workingjubilee`` ``@rbtcollins``  ``@nrc`` ``@fitzgen`` ``@sophiajt`` ``@tmiasko`` ``@notriddle`` ``@TimNN`` ``@WaffleLapkin``
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 1, 2025
Rollup merge of rust-lang#132474 - Noratrieb:lots-of-mailmapping-round-2, r=Mark-Simulacrum

Add more mailmap entries

If you have been pinged: I'm adding a mailmap entry for you to keep thanks.rust-lang.org working properly.

**I have picked the canonical address mostly arbitrarily. If you want a different one as the canonical address, please tell me here**.

<details>
<summary>more details on the change</summary>

builds on top of rust-lang#132471, this containing the less obvious changes that add new canonical email addresses instead of just adding new current ones.

The people updated in this commit have contributed under different email
addresses than the ones they have used in rust-lang/team.
rust-lang/thanks#53 will use team data for thanks reviewers, which requires this to be in
sync.
Therefore, I have updated many of the people that I've noticed being
duplicated after the change.

</details>

I'm adding a novel entry for you: ``@apiraino`` ``@KodrAus`` ``@cramertj`` ``@sunfishcode`` ``@Eh2406`` ``@skade`` ``@huonw`` ``@jsha`` ``@shepmaster`` ``@workingjubilee`` ``@rbtcollins``  ``@nrc`` ``@fitzgen`` ``@sophiajt`` ``@tmiasko`` ``@notriddle`` ``@TimNN`` ``@WaffleLapkin``
@Noratrieb Noratrieb marked this pull request as ready for review January 1, 2025 10:55
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

I played with this locally and it seems to work great. I ran a script to check for possible regressions, and there are many more improvements (entries deduplicated) than regressions (entries duplicated).

We still kind of ignore reviewers without an email in team, but that's unrelated to this PR.

Thanks @Noratrieb for this, it's awesome! And sorry that it took so long to review.

@Kobzol
Copy link
Contributor

Kobzol commented Jan 1, 2025

...after taking into the regressions a bit more, I realize that there were actually quite a bit of regressions based on missing mailmap entries. I implemented an additional deduplication pass that assumes that an e-mail address uniquely identifies a given person, and merges entries based on the same e-mail. The name of the merged entry is based on a heuristic - the name that got the most commits "wins". If people want to change their canonical name, they should update the mailmap.

src/site.rs Show resolved Hide resolved
Copy link
Member Author

@Noratrieb Noratrieb left a comment

Choose a reason for hiding this comment

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

approval

src/site.rs Show resolved Hide resolved
@Kobzol Kobzol merged commit c548f6b into rust-lang:master Jan 1, 2025
1 check passed
@Noratrieb Noratrieb deleted the dyn-reviewers branch January 1, 2025 14:40
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.

Update / autogenerate the reviewer list
5 participants