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

Remove duplicate uids #2254

Merged
merged 1 commit into from
May 27, 2017
Merged

Remove duplicate uids #2254

merged 1 commit into from
May 27, 2017

Conversation

guardrex
Copy link
Contributor

@guardrex guardrex commented May 25, 2017

Fixes #2243

@mairaw I ran each script once on the repo to achieve this effect.

Script 1: https://gist.github.com/GuardRex/29bc96a0f5cd0f3e7b2c9cc4036ab4f5
Script 2: https://gist.github.com/GuardRex/a7e27e558e943094347573d0c47da699

Script 1 orders the entries (descending by length) so that when Distinct (with an IEqualityComparer to filter on the uid) keeps the first one, it's keeping the longer one, which is the one with the ms.author and manager. To preserve the original order, it doesn't write out the unique entries at that point. It uses the unique entries to filter and write out the original list; therefore, the original order is preserved.

Script 2 simply uses Distinct to obtain the unique entries. The result is written directly back to the file, because the order wasn't changed.

@mairaw
Copy link
Contributor

mairaw commented May 26, 2017

It's not showing here but PR has failed. I'll try again.

Copy link
Contributor

@mairaw mairaw left a comment

Choose a reason for hiding this comment

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

I think that this looks great! and brings our warnings to under 1k! :shipit:

@mairaw mairaw merged commit 4b752d7 into dotnet:master May 27, 2017
@mairaw mairaw deleted the guardrex/remove-dup-uids2 branch May 27, 2017 01:10
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.

3 participants