Checker mergequeue compatibility#163
Merged
snowystinger merged 40 commits intomasterfrom Mar 13, 2025
Merged
Conversation
solimant
reviewed
Sep 20, 2024
solimant
reviewed
Sep 20, 2024
checker/checker.js
Outdated
| const ow = openwhisk(); | ||
|
|
||
| if (isMergeQueue) { | ||
| const res = await set_green_is_bot(ow, { |
Contributor
There was a problem hiding this comment.
We actually don't wanna call set_green_is_bot because this handles when a PR is opened by the bot, and will end up giving back this result, like what happened here:
Pull request issued by a bot account, carry on.
Instead, we should add a dedicated, but similar function, like so:
async function set_green_has_signed_cla (ow, args) {
let result;
try {
result = await ow.actions.invoke({
name: utils.SETGITHUBCHECK,
blocking: true,
result: true,
params: {
installation_id: args.installation_id,
org: args.org,
repo: args.repo,
sha: args.commit_sha,
status: 'completed',
start_time: args.start_time,
conclusion: 'success',
title: 'CLA Signed',
summary: 'A Signed CLA has been found for the GitHub.com user ' + args.user
}
});
} catch (e) {
return utils.action_error(e, 'Error invoking setgithubcheck when CLA was found.');
}
return {
statusCode: 200,
body: result.title
};
}
solimant
reviewed
Sep 20, 2024
|
|
||
| const usernames = lookup_res.body.usernames; | ||
| if (usernames.map(function (item) { return item.toLowerCase(); }).includes(args.user.toLowerCase())) { | ||
| // If the username exists in the response from the lookup action, then we |
Contributor
There was a problem hiding this comment.
Once we add the set_green_has_signed_cla function, we can replace the lines below with the following (198-222):
const check = await set_green_has_signed_cla(ow, args);
return check;
snowystinger
commented
Sep 24, 2024
Member
Author
|
hopefully turn off travis check |
solimant
previously approved these changes
Jan 24, 2025
Contributor
solimant
left a comment
There was a problem hiding this comment.
Looks good, thank you! Just one trivial change.
Co-authored-by: solimant <[email protected]>
solimant
approved these changes
Jan 27, 2025
ktabors
reviewed
Jan 31, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This started as a PR to update the code for the cla bot to support the merge queue feature. That was accomplished by adding some logic and new paths in 'checker.js'
However, in the course of this, we found that the version of node that this code relied on was a deprecated/removed environment in adoberuntime. So I bumped the node version to 18, which also included changes to the API as native support for fetch became a thing.
Unfortunately, the test runner was not supported on newer Node versions and it was written in a (now) less popular testing framework, so I moved us to a more modern version of jest and updated all the unit tests as well.
codecov's tool also wasn't supported anymore, but since this project doesn't need a lot of development, I figured it was ok to omit this for now since it's still reported in the CI logs themselves.
In the course of trying to deploy the changes, we also discovered that Travis CI is no longer supported, so I had to move us to GithubActions. During this, we didn't have the secrets for the integration test accounts anymore. I attempted to create new accounts, however, Github has changed some policies and will lock the accounts, any special requests for the accounts to be unblocked result in temporary reinstatement, then they become locked again.
As a result, I've stopped running the integration tests for now.
Checklist
Related Issue