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

Properly credit contributors who were truncated by GitHub API #31739

Merged

Conversation

mirrorcult
Copy link
Contributor

About the PR

The contributor Plasmaguy brought it to my attention that he was not in the credits of the game, despite having 1 merged PR back in 2022. Proper credit is pretty important, so I took a look:

  • First confirmed that his GitHub account still existed, and did in fact have a merged PR
  • Checked the GitHub API endpoint /contributors for the repo, and noticed that he seemed to be missing
  • Curiously, the final page of that endpoint didn't seem to list -any- contributors with only 1 contribution. That's really weird, so I looked a little harder.

Apparently...

image

ah fuck. We definitely have more than 500 email addresses associated with committers at this point, so GitHub has taken the wise decision to truncate those with less contributions first, meaning possibly hundreds of smaller contributors have been going uncredited ingame for an unknown amount of time!! not ideal. Thank you GitHub!

So, we have to complicate the script a little bit to fix that. The comment goes into depth about what exactly we need to change, but the gist is:

  1. need to pass ?anon=1 to get GitHub to properly include all 'unattributed' emails
  2. check for GitHub noreply emails, and extract contributor usernames from that
  3. check all non-GitHub emails associated names, and check if they're a valid GitHub account, then get username from that

This PR also updates the credits list manually. I'll also run the workflow later just to make sure the env var stuff I did works.

As mentioned in the comment it's possible this new approach misattributes some accounts -- this happens when a committer email has a name attached which is a valid GitHub account, e.g. Paul, but this GitHub account is not actually owned by the contributor in question. Not that big of a deal, easy to fix, tried to fix some basic ones I saw, but probably some I missed. Better to overcredit slightly than to undercredit.

Should note that it's unlikely that all of these contributors have -always- remained uncredited. Only once we hit >500 committer emails did the list start getting truncated, so older contributors were probably credited for a length of time before being accidentally removed at some point.

A full list of newly-credited contributors is included here:

List 12rabbits 13spacemen 3nderall 4310v343k abregado Absolute-Potato achookh actually-reb ada-please Adeinitas adrian Ady4ik Aexolott africalimedrop alexkar598 Alithsko ALMv1 AlphaQwerty Andre19926 AndrewEyeke AndreyCamper Anzarot121 Appiah ar4ill areitpog Arkanic armoks ArthurMousatov ArtisticRoomba artur ArZarLordOfMango AsikKEsel astriloqua AutoOtter Awlod backetako Baptr0b0t bellwetherlogic benev0 benjamin-burges bhespiritu bibbly bingojohnson BismarckShuffle BlitzTheSquishy bloodrizer Bloody2372 BobTheSleder boiled-water-tsar botanySupremist BramvanZijp BriBrooo bryce0110 BubblegumBlue buletsponge buntobaggins bvelliquette byondfuckery c0rigin Caconym27 Callmore capnsockless Carolyn3114 Carou02 carteblanche4me ChaseFlorom cheeseplated christhirtle Chubbygummibear civilCornball Clement-O clyf CMDR-Piboy314 Cohnway Colin-Tel CookieMasterT coolboy911 Coolsurf6 CormosLemming croilbird CrzyPotato DadeKuma DakotaGay DanSAussieITS Daracke Daxxi3 dean Deatherd Decortex degradka DenisShvalov derek dersheppard Deserty0 Detintinto DevilishMilk dexlerxd digitalic Dimastra DinoWattz DjfjdfofdjfjD doc-michael docnite dolgovmi dragonryan06 drakewill-CRL Drayff dreamlyjack DrEnzyme dribblydrone drongood12 DrSingh dukevanity duskyjay dvir001 Dynexust echo eden077 efzapa elsie Elysium206 emmafornash eris erohrs2 esguard eugene f0x-n3rd FacePluslll FATFSAAM2 Feluk6174 Fiftyllama FinnishPaladin FirinMaLazors flashgnash FL-OZ FluidRock foboscheshir ForestNoises forgotmyotheraccount forkeyboards forthbridge Fouin foxhorn freeze2222 Froffy025 froozigiusz FrostMando FunTust Futuristic-OK Gaxeer genderGeometries GeneralGaws githubuser508 GlassEclipse godisdeadLOL Goldminermac GraniteSidewalk GreaseMonk greenrock64 GTRsound hamurlik HappyRoach he1acdvv Hebi Henry hiucko Hmeister-fake Hobbitmax hobnob HoidC Holinka4ever holyssss Hyenh icesickleone iczero iglov i-justuser-i illersaver Ilushkins33 imrenq imweax indeano IntegerTempest Itzbenz iztokbajcar jacksonzck Jackw2As jacob Jark255 Jaskanbe JasperJRoth jerryimmouse JimGamemaster jimmy12or jjtParadox justdie12 justin justintether justtne k3yw Kadeo64 Kaga-404 katzenminer kbailey-git Keelin KEEYNy keikiru Kimpes Kirillcas Kistras klaypexx Kmc2000 kognise kokoc9n KonstantinAngelov KrasnoshchekovPavel Kupie kyupolaris kzhanik lajolico larryrussian lawdog4817 Lazzi0706 leander-0 leonardo-dabepis leonsfriedrich LeoSantich lettern LEVELcat lgruthes LightVillet LinkUyx lizelive localcc lokachop LudwigVonChesterfield luizwritescode luminight Lyndomen lyroth001 M3739 mac6na6na magmodius malchanceux MaloTV Markek1 max MaxNox7 maylokana MendaxxDev Mephisto72 MetalSage MFMessage michaelcu micheel665 milon mkanke-real MLGTASTICa moderatelyaware modern-nm mokiros mr-bo-jangles MrFippik MWKane nails-n-tape Nannek NazrinNya neutrino-laser NIXC NkoKirkto noctyrnal NonchalantNoob NoobyLegion not-gavnaed notsodana noverd NuclearWinter nukashimika nullarmo Nylux Nyranu och-och OliverOtter OnyxTheBrave OrangeMoronage9622 Ostaf othymer OttoMaticode packmore paigemaeforrest panzer-iv1 partyaddict PeterFuto PetMudstone pewter-wiz Pgriha Phill101 phunnyguy PilgrimViis Pill-U Pireax Pissachu PixeltheAertistContrib Plasmaguy plinyvic pok27 portfiend PotentiallyTom ProPandaBear Pspritechologist PuceTint Putnam3145 QueerNB RadioMull Raitononai Ramlik randy10122 Ranger6012 Rapidgame7 Redfire1331 Renlou retequizzle rich-dunne riggleprime RIKELOLDABOSS rinary1 riolume RobbyTheFish Rohesie rok-povsic RomanNovo Roudenn router S1rFl0 S1ss3l saga3152 Salex08 sam samgithubaccount SapphicOverload SaveliyM360 sBasalto scruq445 scuffedjays ScumbagDog Segonist sephtasm sewerpig sh18rw ShadeAware shaeone shariathotpatrol siigiil sirdragooon Sk1tch SkaldetSkaeg Slyfox333 snicket sniperchance SolidusSnek Soundwavesghost SpaceyLady spartak SpartanKadence SplinterGP sporekto stanberytrask Stanislav4ix StanTheCarpenter stellar-novas stopbreaking stopka-html Stray-Pyramid strO0pwafel Strol20 StStevens Subversionary sunbear-dev Supernorn Sybil SYNCHRONIC Tainakov Taran taurie telyonok terezi4real texcruize TGODiamond TGRCdev ThatOneGoblin25 thecopbennet TheCze TheDarkElites TheEmber TheIntoxicatedCat thekilk TherapyGoth thevinter TheWaffleJesus ThunderBear2006 timothyteakettle timurjavid tin-man-tim tk-a369 Tollhouse Tonydatguy topy TotallyLemon tropicalhibi truepaintgit Truoizys Tryded tyashley ubis1 Unbelievable-Salmon underscorex5 UnicornOnLSD Unisol unusualcrow user424242420 valentfingerov VelonacepsCalyxEggs veprolet veritable-calamity vero5123 violet754 VMSolidus voidnull000 volotomite vorkathbruh Warentan Watermelon914 wertanchik WilliamECrew willicassi wirdal wtcwr68 xkreksx YanehCheck youarereadingthis Yousifb26 youtissoum Zadeon zamp ZeroDiamond ZeWaka zlodo zzylex

Technical Details

The only major change is that the dump_github_contributors.ps1 script now requires a GitHub API token to run, since the unauthorized rate limit is 60 (!!) per hour and it was hitting that with the new calls to the /users endpoint.

Not an issue for the weekly action, since we can expose GITHUB_TOKEN and use it for that purpose, but if you're looking to run this script yourself for debugging purposes, take note of that (and also that it's going to run quite a bit slower now).

Requirements

  • I have read and I am following the Pull Request Guidelines. I understand that not doing so may get my pr closed at maintainer’s discretion
  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

@github-actions github-actions bot added the Changes: No C# Changes: Requires no C# knowledge to review or fix this item. label Sep 2, 2024
Copy link
Member

@VasilisThePikachu VasilisThePikachu left a comment

Choose a reason for hiding this comment

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

Huh... So this was an issue when it was first pointed out to me... But then was told it was not and just kinda believed it...

Anyway looks good from my side. I will get someone with a better knowledge of PowerShell to check it out too.

@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Sep 8, 2024
Copy link
Contributor

github-actions bot commented Sep 8, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Sep 8, 2024
@Emisse
Copy link
Contributor

Emisse commented Sep 9, 2024

whatever. go my scarabs

@Emisse Emisse merged commit 7a6efea into space-wizards:master Sep 9, 2024
11 checks passed
VMSolidus pushed a commit to Simple-Station/Einstein-Engines that referenced this pull request Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: No C# Changes: Requires no C# knowledge to review or fix this item.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants