GODRIVER-3008 Add mongodb-drivers-comment-bot support #361
Conversation
|
@baileympearson I added you as a reviewer since this is a node app and it involves bootstrapping nvm. |
| mkdir -p $NVM_DIR | ||
| curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.5/install.sh | bash | ||
| [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" | ||
| nvm install lts/hydrogen |
There was a problem hiding this comment.
So - we don't actually use nvm in CI anymore. nvm installs globally by default, which modifies the evergreen host the task is running on. nvm on non-windows machines has a way to work around this, but I think windows does not (we switched off of using nvm sometime last year for this reason). So we maintain our own script to download the latest LTS build of a given Node major version: https://github.com/mongodb/node-mongodb-native/blob/main/.evergreen/install-dependencies.sh. We then use https://github.com/mongodb/node-mongodb-native/blob/main/.evergreen/init-node-and-npm-env.sh to add node and npm to the path when we need them.
If you're not running these on windows, you might be alright. But we could also consider moving these scripts into drivers-evergreen-tools (which would be a useful improvement for Node because we currently maintain like 10 different copies of these scripts across all the repos and branches we support. No exaggeration lol).
What do you think of that idea? I'll want to make sure this is alright with the Node team but I think that initially
- we can just copy these scripts into drivers-evergreen-tools as-is and you can consume them in this PR
- Node will continue to maintain them
- the Node team can eventually adopt them and transition our CI to use them
There was a problem hiding this comment.
Oh nice, I like the idea of copying over those scripts as part of this PR, independent of what your team ends up doing. I had intended this script to be run on a Linux host, but using the "standard" approach sounds better overall.
There was a problem hiding this comment.
I made a couple of tweaks: the default LTS is 18, and it uses local paths instead of PROJECT_DIRECTORY. I'm testing it now again.
| resp.data.forEach(async (comment) => { | ||
| if (comment.body.includes(bodyMatch)) { | ||
| if (found) { | ||
| return | ||
| } | ||
| found = true; | ||
| await octokit.request("PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}", { | ||
| owner, | ||
| repo, | ||
| body: bodyText, | ||
| comment_id: comment.id, | ||
| headers | ||
| }); | ||
| } | ||
| }) |
There was a problem hiding this comment.
In Node, you need to be careful of using async functions as callbacks where they're not expected, because if the async function throws an error, it won't be caught and the program crashes. Here I'd either use Array.find() again, or use a for-of loop
| resp.data.forEach(async (comment) => { | |
| if (comment.body.includes(bodyMatch)) { | |
| if (found) { | |
| return | |
| } | |
| found = true; | |
| await octokit.request("PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}", { | |
| owner, | |
| repo, | |
| body: bodyText, | |
| comment_id: comment.id, | |
| headers | |
| }); | |
| } | |
| }) | |
| const comment = resp.data.find(comment => comment.body.includes(bodyMatch)); | |
| if (!comment) { | |
| // create comment | |
| } | |
| // update comment |
or
for (const comment of resp.data)
if (comment.body.includes(bodyMatch)) {
found = true;
await octokit.request("PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}", {
owner,
repo,
body: bodyText,
comment_id: comment.id,
headers
});
break;
}
}
Co-authored-by: Bailey Pearson <[email protected]>
Co-authored-by: Bailey Pearson <[email protected]>
Co-authored-by: Bailey Pearson <[email protected]>
|
All green again on both sides! |
Used with mongodb/mongo-go-driver#1424 to create an API report comment: