Add handling for when author is null #38
Conversation
|
Everyone contributing to this PR have now signed the CLA. Thanks! |
|
The funny thing is that I DID sign the CLA, or at least I thought I did, back when I made this PR canonical/lightdm#340 which is where I first noticed this cla-check erroneously passing. I do indeed have a launchpad ID that I set up when I created a Ubuntu One account in the process of getting the CLA stuff done, but I either did it wrong, or need to do it again.. but I don't want to fix it, and break my working examples of this cla-check bug. 🙃 (or I can try and find some odd commit to rebase and trick CI into passing 😉) |
|
@wallentx how will it work in case if we have several people with |
* Update index.js * Update index.js * Update index.js
|
@beliaev-maksim I apologize for the delay in response. I believe that it should have handled multiple Additionally, it was not deduplicating the list of contributors, and would make multiple calls for each entry. I made some changes to consolidate everything into a map. I also injected some fake user contributions into the list on a separate test branch to see how it handled multiple Oh, I also included conditional debug logging so that it prints the contributor list that it is working against (like the output in the run above) that is only executed if the github actions run is in debug mode. Some of that is extra, so feel free to make whatever changes you feel, as you see fit. |
index.js
Outdated
| core.startGroup('Installing python3-launchpadlib'); | ||
| await exec.exec('sudo apt-get update'); | ||
| await exec.exec('sudo apt-get install python3-launchpadlib'); | ||
| core.endGroup() | ||
| core.endGroup(); |
There was a problem hiding this comment.
please move all the styling changes into a separate PR
There was a problem hiding this comment.
@beliaev-maksim done! plz let me know if anything still looks opinionated
| try { | ||
| await ghRepo.request('GET /users/' + username); | ||
| } catch (error) { | ||
| console.log('- ' + username + ' ✕ (GitHub user does not exist)'); | ||
| continue; |
There was a problem hiding this comment.
why do we need this part ?
| passed = false; | ||
| non_signers.push(i) | ||
| break; |
There was a problem hiding this comment.
why do not we break anymore ?
|
@wallentx
if that is submitted as 4 separate PRs, then we can do the review a way faster for each of the PRs Now, the burden is on the repo maintainers to understand which changes belong to which group. |
There are some very specific conditions where a user who has not signed the CLA can still have their PR pass the cla-check workflow if they rebase, and push commits that were authored by someone who has signed the CLA, but their author information returns
null.wallentx/lightdm#3
https://github.com/wallentx/lightdm/pull/3/commits
In the above example, the true contributor details are:
[ { "username": "wallentx", "email": "[email protected]", "signed": false }, { "username": null, "email": "[email protected]", "signed": false }, { "username": "madpilot78", "email": "[email protected]", "signed": false }, { "username": "n3rdopolis", "email": "[email protected]", "signed": false } ]But without handling of
null, the list gets deformed, and is processed as:[ { "username": "wallentx", "email": "[email protected]", "signed": false }, { "username": "madpilot78", "email": "[email protected]", "signed": false }, { "username": "n3rdopolis", "email": "[email protected]", "signed": false } ]And so during the CI run, you see:
The GitHub API is unable to derive the GitHub username for
guido(https://github.com/gber) from the query being used (I don't actually know if there's a GH API endpoint that will allow you to discover that if their commit can't be associated with a REF), so theauthorinformation shows up asnull, which I guess the prior record ends up taking its place. my email address is never checked against Launchpad, because it slotted me down into the null spot.This check should not have passed, because I have never signed the CLA.
https://github.com/wallentx/lightdm/actions/runs/8579267328/job/23514827215?pr=3