damccorm commented on a change in pull request #16898: URL: https://github.com/apache/beam/pull/16898#discussion_r814239064
########## File path: scripts/ci/pr-bot/shared/userCommand.ts ########## @@ -0,0 +1,231 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const github = require("./githubUtils"); +const commentStrings = require("./commentStrings"); +const { BOT_NAME } = require("./constants"); +const { StateClient } = require("./persistentState"); +const { ReviewerConfig } = require("./reviewerConfig"); + +// Reads the comment and processes the command if one is contained in it. +// Returns true if it runs a command, false otherwise. +export async function processCommand( + payload: any, + commentAuthor: string, + commentText: string, + stateClient: typeof StateClient, + reviewerConfig: typeof ReviewerConfig +) { + // Don't process any commands from our bot. + if (commentAuthor === BOT_NAME) { + return false; + } + console.log(commentAuthor); + + const pullNumber = payload.issue?.number || payload.pull_request?.number; + commentText = commentText.toLowerCase(); + if (commentText.indexOf("r: @") > -1) { + await manuallyAssignedToReviewer(pullNumber, stateClient); + } else if (commentText.indexOf("assign to next reviewer") > -1) { + await assignToNextReviewer( + payload, + commentAuthor, + pullNumber, + stateClient, + reviewerConfig + ); + } else if (commentText.indexOf("stop reviewer notifications") > -1) { + await stopReviewerNotifications( + pullNumber, + stateClient, + "requested by reviewer" + ); + } else if (commentText.indexOf("remind me after tests pass") > -1) { + await remindAfterTestsPass(pullNumber, commentAuthor, stateClient); + } else if (commentText.indexOf("waiting on author") > -1) { + await waitOnAuthor(payload, pullNumber, stateClient); + } else if (commentText.indexOf("assign set of reviewers") > -1) { + await assignReviewerSet(payload, pullNumber, stateClient, reviewerConfig); + } else { + return false; + } + + return true; +} + +async function assignToNextReviewer( + payload: any, + commentAuthor: string, + pullNumber: number, + stateClient: typeof StateClient, + reviewerConfig: typeof ReviewerConfig +) { + let prState = await stateClient.getPrState(pullNumber); + let labelOfReviewer = prState.getLabelForReviewer(payload.sender.login); + if (labelOfReviewer) { + let reviewersState = await stateClient.getReviewersForLabelState( + labelOfReviewer + ); + const pullAuthor = + payload.issue?.user?.login || payload.pull_request?.user?.login; + let availableReviewers = reviewerConfig.getReviewersForLabel( + labelOfReviewer, + [commentAuthor, pullAuthor] + ); + let chosenReviewer = reviewersState.assignNextReviewer(availableReviewers); + prState.reviewersAssignedForLabels[labelOfReviewer] = chosenReviewer; + + // Comment assigning reviewer + console.log(`Assigning ${chosenReviewer}`); + await github.addPrComment( + pullNumber, + commentStrings.assignReviewer(prState.reviewersAssignedForLabels) + ); + + // Set next action to reviewer + const existingLabels = + payload.issue?.labels || payload.pull_request?.labels; + await github.nextActionReviewers(pullNumber, existingLabels); + prState.nextAction = "Reviewers"; + + // Persist state + await stateClient.writePrState(pullNumber, prState); + await stateClient.writeReviewersForLabelState( + labelOfReviewer, + reviewersState + ); + } +} + +// If they've manually assigned a reviewer, just silence notifications and ignore this pr going forward. +// TODO(damccorm) - we could try to do something more intelligent here like figuring out which label that reviewer belongs to. +async function manuallyAssignedToReviewer( + pullNumber: number, + stateClient: typeof StateClient +) { + await stopReviewerNotifications( + pullNumber, + stateClient, + "review requested by someone other than the bot, ceding control" + ); +} + +async function stopReviewerNotifications( + pullNumber: number, + stateClient: typeof StateClient, + reason: string +) { + let prState = await stateClient.getPrState(pullNumber); + prState.stopReviewerNotifications = true; + await stateClient.writePrState(pullNumber, prState); + + // Comment acknowledging command + await github.addPrComment( + pullNumber, + commentStrings.stopNotifications(reason) + ); +} + +async function remindAfterTestsPass( + pullNumber: number, + username: string, + stateClient: typeof StateClient +) { + let prState = await stateClient.getPrState(pullNumber); + prState.remindAfterTestsPass.push(username); + await stateClient.writePrState(pullNumber, prState); + + // Comment acknowledging command + await github.addPrComment( + pullNumber, + commentStrings.remindReviewerAfterTestsPass(username) + ); +} + +async function waitOnAuthor( + payload: any, + pullNumber: number, + stateClient: typeof StateClient +) { + const existingLabels = payload.issue?.labels || payload.pull_request?.labels; + await github.nextActionAuthor(pullNumber, existingLabels); + let prState = await stateClient.getPrState(pullNumber); + prState.nextAction = "Author"; + await stateClient.writePrState(pullNumber, prState); +} + +async function assignReviewerSet( + payload: any, + pullNumber: number, + stateClient: typeof StateClient, + reviewerConfig: typeof ReviewerConfig +) { + let prState = await stateClient.getPrState(pullNumber); + if (Object.values(prState.reviewersAssignedForLabels).length > 0) { + await github.addPrComment( + pullNumber, + commentStrings.reviewersAlreadyAssigned( + Object.values(prState.reviewersAssignedForLabels) + ) + ); + return; + } + + const existingLabels = payload.issue?.labels || payload.pull_request?.labels; + const pullAuthor = + payload.issue?.user?.login || payload.pull_request?.user?.login; + const reviewersForLabels = reviewerConfig.getReviewersForLabels( + existingLabels, + [pullAuthor] + ); + let reviewerStateToUpdate = {}; + var labels = Object.keys(reviewersForLabels); + if (!labels || labels.length == 0) { Review comment: That gets pretty unclear when labels is null, especially because technically the whole condition would be `!(labels?.length > 0)`. For readability, I think I prefer the explicit `||` ########## File path: scripts/ci/pr-bot/shared/reviewersForLabel.ts ########## @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const github = require("./githubUtils"); + +export class ReviewersForLabel { + public label: string; + public dateOfLastReviewAssignment: { [key: string]: number }; + + constructor( + label: string, + propertyDictionary: { + dateOfLastReviewAssignment: { [key: string]: number }; + } + ) { + this.label = label; + this.dateOfLastReviewAssignment = {}; // map of reviewer to date + + if (!propertyDictionary) { + return; + } + if ("dateOfLastReviewAssignment" in propertyDictionary) { + this.dateOfLastReviewAssignment = + propertyDictionary["dateOfLastReviewAssignment"]; + } + } + + // Given a list of available reviewers, + // returns the next reviewer up based on who has reviewed least recently. + // Updates this object to reflect their assignment. + assignNextReviewer(availableReviewers: string[]): string { + if (availableReviewers.length == 0) { + throw new Error(`No reviewers available for label ${this.label}`); + } + + if (!this.dateOfLastReviewAssignment[availableReviewers[0]]) { + this.dateOfLastReviewAssignment[availableReviewers[0]] = Date.now(); + return availableReviewers[0]; + } + + let earliestDate = this.dateOfLastReviewAssignment[availableReviewers[0]]; + let earliestReviewer = availableReviewers[0]; + + for (let i = 0; i < availableReviewers.length; i++) { + let availableReviewer = availableReviewers[i]; + if (!this.dateOfLastReviewAssignment[availableReviewer]) { + this.dateOfLastReviewAssignment[availableReviewer] = Date.now(); + return availableReviewer; + } + if (earliestDate > this.dateOfLastReviewAssignment[availableReviewer]) { + earliestDate = this.dateOfLastReviewAssignment[availableReviewer]; + earliestReviewer = availableReviewer; + } + } + + this.dateOfLastReviewAssignment[earliestReviewer] = Date.now(); + return earliestReviewer; + } + + // Given the up to date list of available reviewers (excluding the author), + // returns the next reviewer up based on who has reviewed least recently. + // Updates this object to reflect their assignment. + assignNextCommitter(availableReviewers: string[]): string { + let earliestDate = Date.now(); + let earliestCommitter: string = ""; + + for (let i = 0; i < availableReviewers.length; i++) { + let availableReviewer = availableReviewers[i]; + if (github.checkIfCommitter(availableReviewer)) { Review comment: Good catch! ########## File path: scripts/ci/pr-bot/shared/userCommand.ts ########## @@ -0,0 +1,231 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const github = require("./githubUtils"); +const commentStrings = require("./commentStrings"); +const { BOT_NAME } = require("./constants"); +const { StateClient } = require("./persistentState"); +const { ReviewerConfig } = require("./reviewerConfig"); + +// Reads the comment and processes the command if one is contained in it. +// Returns true if it runs a command, false otherwise. +export async function processCommand( + payload: any, + commentAuthor: string, + commentText: string, + stateClient: typeof StateClient, + reviewerConfig: typeof ReviewerConfig +) { + // Don't process any commands from our bot. + if (commentAuthor === BOT_NAME) { + return false; + } + console.log(commentAuthor); + + const pullNumber = payload.issue?.number || payload.pull_request?.number; + commentText = commentText.toLowerCase(); + if (commentText.indexOf("r: @") > -1) { + await manuallyAssignedToReviewer(pullNumber, stateClient); + } else if (commentText.indexOf("assign to next reviewer") > -1) { + await assignToNextReviewer( + payload, + commentAuthor, + pullNumber, + stateClient, + reviewerConfig + ); + } else if (commentText.indexOf("stop reviewer notifications") > -1) { + await stopReviewerNotifications( + pullNumber, + stateClient, + "requested by reviewer" + ); + } else if (commentText.indexOf("remind me after tests pass") > -1) { + await remindAfterTestsPass(pullNumber, commentAuthor, stateClient); + } else if (commentText.indexOf("waiting on author") > -1) { + await waitOnAuthor(payload, pullNumber, stateClient); + } else if (commentText.indexOf("assign set of reviewers") > -1) { + await assignReviewerSet(payload, pullNumber, stateClient, reviewerConfig); + } else { + return false; + } + + return true; +} + +async function assignToNextReviewer( + payload: any, Review comment: I skipped this one because we don't have the ability to intelligently cast this later on. This really is an `any` value because it represents a payload with potentially different forms - its much easier to pass it around like that than continually try to cast it to different things everywhere we want to dereference a value (which basically loses any type safety anyways) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
