[GitHub] [beam] tvalentyn commented on a diff in pull request #21791: Expand pr bot to python

[GitBox <gi...@apache.org>]

tvalentyn commented on code in PR #21791:
URL: https://github.com/apache/beam/pull/21791#discussion_r899064067


##########
scripts/ci/pr-bot/processNewPrs.ts:
##########
@@ -45,15 +45,24 @@ import { CheckStatus } from "./shared/checks";
  * (in which case that's all we need to do).
  */
 function needsProcessed(pull: any, prState: typeof Pr): boolean {
-  if (!pull.labels.find((label) => label.name.toLowerCase() === "go")) {
+  if (
+    !pull.labels.find(
+      (label) =>
+        label.name.toLowerCase() === "go" ||
+        label.name.toLowerCase() == "python"

Review Comment:
   nit: probably doens't make a difference, but there is strict equality above



##########
scripts/ci/pr-bot/processPrUpdate.ts:
##########
@@ -136,8 +136,14 @@ async function processPrUpdate() {
 
   // TODO(damccorm) - remove this when we roll out to more than go
   const existingLabels = payload.issue?.labels || payload.pull_request?.labels;
-  if (!existingLabels.find((label) => label.name.toLowerCase() === "go")) {
-    console.log("Does not contain the go label - skipping");
+  if (
+    !existingLabels.find(
+      (label) =>
+        label.name.toLowerCase() === "go" ||
+        label.name.toLowerCase() === "python"
+    )
+  ) {
+    console.log("Does not contain the go or python labels - skipping");

Review Comment:
   for my education, where can these logs be inspected?



##########
scripts/ci/pr-bot/processNewPrs.ts:
##########
@@ -45,15 +45,24 @@ import { CheckStatus } from "./shared/checks";
  * (in which case that's all we need to do).
  */
 function needsProcessed(pull: any, prState: typeof Pr): boolean {
-  if (!pull.labels.find((label) => label.name.toLowerCase() === "go")) {
+  if (
+    !pull.labels.find(
+      (label) =>
+        label.name.toLowerCase() === "go" ||
+        label.name.toLowerCase() == "python"
+    )
+  ) {
     console.log(
-      `Skipping PR ${pull.number} because it doesn't contain the go label`
+      `Skipping PR ${pull.number} because it doesn't contain the go or python labels`
     );
     return false;
   }
-  const firstPrToProcess = new Date(2022, 2, 2, 20);
+  const firstPrToProcess = new Date(2022, 7, 16, 14); // June 16 2022, 14:00 UTC (note that Java months are 0 indexed)

Review Comment:
   did you mean to use (2022, 5, 16, 14) if they are 0-indexed?



##########
scripts/ci/pr-bot/processNewPrs.ts:
##########
@@ -45,15 +45,24 @@ import { CheckStatus } from "./shared/checks";
  * (in which case that's all we need to do).
  */
 function needsProcessed(pull: any, prState: typeof Pr): boolean {
-  if (!pull.labels.find((label) => label.name.toLowerCase() === "go")) {
+  if (

Review Comment:
   re item 6) above : for my education, what does it mean to have notifications stopped?
   item 7 is out of sync now.



##########
scripts/ci/pr-bot/processNewPrs.ts:
##########
@@ -45,15 +45,24 @@ import { CheckStatus } from "./shared/checks";
  * (in which case that's all we need to do).
  */
 function needsProcessed(pull: any, prState: typeof Pr): boolean {
-  if (!pull.labels.find((label) => label.name.toLowerCase() === "go")) {
+  if (
+    !pull.labels.find(
+      (label) =>
+        label.name.toLowerCase() === "go" ||
+        label.name.toLowerCase() == "python"
+    )
+  ) {
     console.log(
-      `Skipping PR ${pull.number} because it doesn't contain the go label`
+      `Skipping PR ${pull.number} because it doesn't contain the go or python labels`
     );
     return false;
   }
-  const firstPrToProcess = new Date(2022, 2, 2, 20);
+  const firstPrToProcess = new Date(2022, 7, 16, 14); // June 16 2022, 14:00 UTC (note that Java months are 0 indexed)
   const createdAt = new Date(pull.created_at);
-  if (createdAt < firstPrToProcess) {
+  if (
+    createdAt < firstPrToProcess &&
+    !pull.labels.find((label) => label.name.toLowerCase() === "go")

Review Comment:
   do we need to exclude `go` here given that the bot has already been enabled for go? and probably all outstanding `go` prs have been processed already, except for maybe a couple go of PRs that happen to be submitted before two successive iteration of this script?



-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org