Skip to content

Fix cla 504#51

Merged
gitcommitshow merged 9 commits intomainfrom
fix-cla-504
Sep 27, 2024
Merged

Fix cla 504#51
gitcommitshow merged 9 commits intomainfrom
fix-cla-504

Conversation

@gitcommitshow
Copy link
Copy Markdown
Owner

No description provided.

@gitcommitshow gitcommitshow linked an issue Sep 25, 2024 that may be closed by this pull request

- [x] When an external contributor (not the internal team) raises a PR, post a comment to sign CLA and label PR `Pending CLA`
- [x] On signing CLA, remove `Pending CLA` label and never ask that user to sign the CLA again on any of our repo in future
- [x] On signing CLA, remove `Pending CLA` label from all the PRs of that user. Never ask that user to sign the CLA on any of our repo in future
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Suggested change
- [x] On signing CLA, remove `Pending CLA` label from all the PRs of that user. Never ask that user to sign the CLA on any of our repo in future
- [x] On signing CLA, remove `Pending CLA` label from all the open PRs created by that contributor. Never ask that contributor to sign the CLA on any of our repo in future

src/helpers.js Outdated
const filteredPrs = prs.filter(pr => pr.user.login === githubUsername);
console.log(`Found ${filteredPrs.length} open PRs for ${githubUsername} in ${org}:`, filteredPrs.map(pr => pr.number).join(', '));

for await (const { installation } of app.eachInstallation.iterator()) {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Is this really needed?

  1. Why not use the app.octokit instance for removeLabel call?
  2. Is installationId (and hence auth scope) different for different repo of the same org?
  3. Can we have a more unified way to get the correct authenticated octokit instance? e.g. getAuthenticatedInstance(repo, type)

src/helpers.js Outdated
const hasPendingCLALabel = pr.labels?.some(label => label?.name?.toLowerCase() === "pending cla");
console.log(`PR #${pr.number} has "Pending CLA" label: ${hasPendingCLALabel}`);
if (hasPendingCLALabel) {
await removePendingCLALabel(app.octokit, org, repository.name, pr.number);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Suggested change
await removePendingCLALabel(app.octokit, org, repository.name, pr.number);
await removePendingCLALabel(app.octokit, org, pr?.repository?.name, pr.number);

src/helpers.js Outdated
if (hasPendingCLALabel) {
await removePendingCLALabel(app.octokit, org, repository.name, pr.number);
} else {
console.log(`PR #${pr.number} in ${org}/${repository.name} does not have "Pending CLA" label. Skipping.`);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Suggested change
console.log(`PR #${pr.number} in ${org}/${repository.name} does not have "Pending CLA" label. Skipping.`);
console.log(`PR #${pr.number} in ${org}/${pr?.repository.name} does not have "Pending CLA" label. Skipping.`);

@gitcommitshow gitcommitshow merged commit 9c017bc into main Sep 27, 2024
@gitcommitshow gitcommitshow deleted the fix-cla-504 branch September 27, 2024 05:49
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.

504 gateway timeout error after signing successfully

1 participant