You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/02/18 17:31:22 UTC
[GitHub] [beam] damccorm opened a new pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
damccorm opened a new pull request #16898:
URL: https://github.com/apache/beam/pull/16898
Right now, we don't have a well defined automated system for staying on top of pull request reviews - we rely on contributors being able to find the correct OWNERS file and committers manually triaging/calling attention to old pull requests. This is the start of an effort to improve our automation around this. [Design doc for the full effort here](https://docs.google.com/document/d/1FhRPRD6VXkYlLAPhNfZB7y2Yese2FCWBzjx67d3TjBo/edit#)
I have the code to handle new prs and pr updates in a repo here - https://github.com/damccorm/beam-pr-bot-demo - this is a subset of the [full set of changes](https://github.com/damccorm/beam-pr-bot-demo/pull/1) split up to try to keep reviews manageable. This Pr introduces most of the shared files used by the automations for things like interacting with GitHub, storing/interacting with our durable state on a dedicated branch and configuration, and some constants used by the automations.
Initially, I am filtering all bot activity prs with the Go label. This is to minimize the blast radius of the automation in case it doesn't work perfectly out the gate or we find it unhelpful. Eventually, I plan on expanding to the rest of the repo.
You can see see the completed portions of the bot in action in [this pr](https://github.com/damccorm/beam-pr-bot-demo/pull/3) and [this pr](https://github.com/damccorm/beam-pr-bot-demo/pull/4) (they show different use cases and exercise almost all of the behavior of the bot).
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [x] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
`ValidatesRunner` compliance status (on master branch)
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/badge/icon?subject=V1+Streaming">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon?subject=V1+Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/badge/icon?subject=V2+Streaming">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon?subject=Java+8">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon?subject=Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon?subject=Portable+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/badge/icon?subject=Portable">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon?subject=Structured+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon?subject=ValCont">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_PythonUsingJava_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_PythonUsingJava_Dataflow/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_PythonUsingJavaSQL_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_PythonUsingJavaSQL_Dataflow/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_JavaUsingPython_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_JavaUsingPython_Dataflow/lastCompletedBuild/badge/icon">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
</tbody>
</table>
Examples testing status on various runners
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/badge/icon?subject=V1+Java11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
Post-Commit SDK/Transform Integration Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>Go</th>
<th>Java</th>
<th>Python</th>
</tr>
</thead>
<tbody>
<tr>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon?subject=3.6">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon?subject=3.7">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon?subject=3.8">
</a>
</td>
</tr>
</tbody>
</table>
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>---</th>
<th>Java</th>
<th>Python</th>
<th>Go</th>
<th>Website</th>
<th>Whitespace</th>
<th>Typescript</th>
</tr>
</thead>
<tbody>
<tr>
<td>Non-portable</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon?subject=Tests">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon?subject=Lint">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon?subject=Docker">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon?subject=Docs">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Portable</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813176394
##########
File path: scripts/ci/pr-bot/README.md
##########
@@ -0,0 +1,45 @@
+<!--
+ 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.
+-->
+
+# Pr Bot
+
+This directory holds all the code (except for Actions Workflows) for our pr bot designed to improve the pr experience.
+For a list of commands to use when interacting with the bot, see [Commands.md](./Commands.md).
+For a design doc explaining the design and implementation, see [Automate Reviewer Assignment](https://docs.google.com/document/d/1FhRPRD6VXkYlLAPhNfZB7y2Yese2FCWBzjx67d3TjBo/edit#)
+
+## Build/Test
+
+To build, run:
+
+```
+npm install
+npm run build
+```
+
+To run the tests:
+
+```
+npm test
+```
+
+Before checking in code, run prettier on it:
+
+```
+npm run format
Review comment:
Prettier should eventually be added as a [pre-commit](https://pre-commit.ci/).
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813195512
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
+}
+
+export async function addPrComment(pullNumber: number, body: string) {
+ const githubClient = getGitHubClient();
+ await githubClient.rest.issues.createComment({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ body: body,
+ });
+}
+
+export async function nextActionReviewers(
+ pullNumber: number,
+ existingLabels: any[]
+) {
+ const client = getGitHubClient();
Review comment:
Don't store the client to save line space.
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813268116
##########
File path: scripts/ci/pr-bot/shared/commentStrings.ts
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+export function allChecksPassed(reviewersToNotify: string[]): string {
+ return `All checks have passed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function assignCommitter(committer: string): string {
+ return `R: @${committer} for final approval`;
+}
+
+export function assignReviewer(labelToReviewerMapping: any): string {
+ let commentString =
+ "Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:\n\n";
+
+ for (let label in labelToReviewerMapping) {
+ let reviewer = labelToReviewerMapping[label];
+ if (label == "no-matching-label") {
+ commentString += `R: @${reviewer} added as fallback since no labels match configuration\n`;
+ } else {
+ commentString += `R: @${reviewer} for label ${label}.\n`;
+ }
+ }
+
+ commentString += `
+Available commands:
+- \`stop reviewer notifications\` - opt out of the automated review tooling
+- \`remind me after tests pass\` - tag the comment author after tests pass
+- \`waiting on author\` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)`;
+ return commentString;
+}
+
+export function failingChecksCantAssign(): string {
+ return "Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`\"";
Review comment:
No, good catch - updated
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r814216934
##########
File path: scripts/ci/pr-bot/shared/reviewerConfig.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 yaml = require("js-yaml");
+const fs = require("fs");
+import { Label } from "./githubUtils";
+
+export class ReviewerConfig {
+ private config: any;
+ constructor(pathToConfigFile) {
+ this.config = yaml.load(
+ fs.readFileSync(pathToConfigFile, { encoding: "utf-8" })
+ );
+ }
+
+ // Given a list of labels and an exclusion list of reviewers not to include (e.g. the author)
+ // returns all possible reviewers for each label
+ getReviewersForLabels(
+ labels: Label[],
+ exclusionList: string[]
+ ): { [key: string]: string[] } {
+ let reviewersFound = false;
+ let labelToReviewerMapping = {};
+ labels.forEach((label) => {
+ let reviewers = this.getReviewersForLabel(label.name, exclusionList);
+ if (reviewers.length > 0) {
+ labelToReviewerMapping[label.name] = reviewers;
+ reviewersFound = true;
+ }
+ });
+ if (!reviewersFound) {
+ const fallbackReviewers = this.getFallbackReviewers(exclusionList);
+ if (fallbackReviewers.length > 0) {
+ labelToReviewerMapping["no-matching-label"] =
+ this.getFallbackReviewers(exclusionList);
+ }
+ }
+ return labelToReviewerMapping;
+ }
+
+ // Get possible reviewers excluding the author.
+ getReviewersForLabel(label: string, exclusionList: string[]): string[] {
+ var labelObjects = this.config.labels;
+ for (var i = 0; i < labelObjects.length; i++) {
+ var labelObject = labelObjects[i];
+ if (labelObject.name.toLowerCase() === label.toLowerCase()) {
+ return this.excludeFromReviewers(labelObject.reviewers, exclusionList);
+ }
+ }
+ return [];
Review comment:
Agreed - good call! Updated here (and the similar one below)
##########
File path: scripts/ci/pr-bot/shared/persistentState.ts
##########
@@ -0,0 +1,123 @@
+/*
+ * 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 exec = require("@actions/exec");
+const fs = require("fs");
+const path = require("path");
+const { Pr } = require("./pr");
+const { ReviewersForLabel } = require("./reviewersForLabel");
+const { BOT_NAME } = require("./constants");
+
+export class PersistentState {
+ private switchedBranch: boolean;
+
+ constructor() {
+ this.switchedBranch = false;
+ }
+
+ // Returns a Pr object representing the current saved state of the pr.
+ async getPrState(prNumber: number): Promise<any> {
+ var fileName = this.getPrFileName(prNumber);
+ return new Pr(await this.getState(fileName, "state/pr-state"));
+ }
+
+ // Writes a Pr object representing the current saved state of the pr to persistent storage.
+ async writePrState(prNumber: number, newState: any) {
+ var fileName = this.getPrFileName(prNumber);
+ await this.writeState(fileName, "state/pr-state", new Pr(newState));
+ }
+
+ // Returns a ReviewersForLabel object representing the current saved state of which reviewers have reviewed recently.
+ async getReviewersForLabelState(label: string): Promise<any> {
+ var fileName = this.getReviewersForLabelFileName(label);
+ return new ReviewersForLabel(label, await this.getState(fileName, "state"));
+ }
+
+ // Writes a ReviewersForLabel object representing the current saved state of which reviewers have reviewed recently.
+ async writeReviewersForLabelState(label: string, newState: any) {
+ var fileName = this.getReviewersForLabelFileName(label);
+ await this.writeState(
+ fileName,
+ "state",
+ new ReviewersForLabel(label, newState)
+ );
+ }
+
+ private async getState(fileName, baseDirectory) {
+ await this.ensureCorrectBranch();
+ fileName = path.join(baseDirectory, fileName);
+ if (!fs.existsSync(fileName)) {
+ return null;
+ }
+ return JSON.parse(fs.readFileSync(fileName, { encoding: "utf-8" }));
+ }
+
+ private async writeState(fileName, baseDirectory, state) {
+ await this.ensureCorrectBranch();
+ fileName = path.join(baseDirectory, fileName);
+ if (!fs.existsSync(baseDirectory)) {
+ fs.mkdirSync(baseDirectory, { recursive: true });
+ }
+ fs.writeFileSync(fileName, JSON.stringify(state, null, 2), {
+ encoding: "utf-8",
+ });
+ await this.commitStateToRepo();
+ }
+
+ private async ensureCorrectBranch() {
+ if (this.switchedBranch) {
+ return;
+ }
+ console.log(
+ "Switching to branch pr-bot-state for reading/storing persistent state between runs"
+ );
+ try {
+ await exec.exec(`git config user.name ${BOT_NAME}`);
+ await exec.exec(`git config user.email ${BOT_NAME}@github.com`);
+ await exec.exec("git config pull.rebase false");
+ await exec.exec("git fetch origin pr-bot-state");
+ await exec.exec("git checkout pr-bot-state");
+ } catch {
+ console.log(
+ "Couldnt find branch pr-bot-state in origin, trying to create it"
+ );
+ try {
+ await exec.exec("git checkout -b pr-bot-state");
+ } catch {
+ console.log("Creating branch failed, trying a simple checkout.");
+ await exec.exec("git checkout pr-bot-state");
+ }
+ }
+ this.switchedBranch = true;
+ }
+
+ private getPrFileName(prNumber) {
Review comment:
I don't necessarily agree with this style decision, but don't want to block on it either - updated
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813181832
##########
File path: scripts/ci/pr-bot/shared/commentStrings.ts
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+export function allChecksPassed(reviewersToNotify: string[]): string {
+ return `All checks have passed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function assignCommitter(committer: string): string {
+ return `R: @${committer} for final approval`;
+}
+
+export function assignReviewer(labelToReviewerMapping: any): string {
+ let commentString =
+ "Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:\n\n";
+
+ for (let label in labelToReviewerMapping) {
+ let reviewer = labelToReviewerMapping[label];
+ if (label == "no-matching-label") {
+ commentString += `R: @${reviewer} added as fallback since no labels match configuration\n`;
+ } else {
+ commentString += `R: @${reviewer} for label ${label}.\n`;
+ }
+ }
+
+ commentString += `
+Available commands:
+- \`stop reviewer notifications\` - opt out of the automated review tooling
+- \`remind me after tests pass\` - tag the comment author after tests pass
+- \`waiting on author\` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)`;
+ return commentString;
+}
+
+export function failingChecksCantAssign(): string {
+ return "Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`\"";
Review comment:
Is the last double quote supposed to be there? It seems like the message will read as below:
If you'd like to override that behavior, comment `assign set of reviewers`"
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813204124
##########
File path: scripts/ci/pr-bot/shared/persistentState.ts
##########
@@ -0,0 +1,123 @@
+/*
+ * 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 exec = require("@actions/exec");
+const fs = require("fs");
+const path = require("path");
+const { Pr } = require("./pr");
+const { ReviewersForLabel } = require("./reviewersForLabel");
+const { BOT_NAME } = require("./constants");
+
+export class PersistentState {
+ private switchedBranch: boolean;
+
+ constructor() {
+ this.switchedBranch = false;
+ }
+
+ // Returns a Pr object representing the current saved state of the pr.
+ async getPrState(prNumber: number): Promise<any> {
Review comment:
Avoid `any` type if you can, perhaps use `unknown` type here.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r814200380
##########
File path: scripts/ci/pr-bot/shared/persistentState.ts
##########
@@ -0,0 +1,123 @@
+/*
+ * 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 exec = require("@actions/exec");
+const fs = require("fs");
+const path = require("path");
+const { Pr } = require("./pr");
+const { ReviewersForLabel } = require("./reviewersForLabel");
+const { BOT_NAME } = require("./constants");
+
+export class PersistentState {
+ private switchedBranch: boolean;
+
+ constructor() {
+ this.switchedBranch = false;
+ }
+
+ // Returns a Pr object representing the current saved state of the pr.
+ async getPrState(prNumber: number): Promise<any> {
+ var fileName = this.getPrFileName(prNumber);
+ return new Pr(await this.getState(fileName, "state/pr-state"));
+ }
+
+ // Writes a Pr object representing the current saved state of the pr to persistent storage.
+ async writePrState(prNumber: number, newState: any) {
+ var fileName = this.getPrFileName(prNumber);
+ await this.writeState(fileName, "state/pr-state", new Pr(newState));
+ }
+
+ // Returns a ReviewersForLabel object representing the current saved state of which reviewers have reviewed recently.
+ async getReviewersForLabelState(label: string): Promise<any> {
+ var fileName = this.getReviewersForLabelFileName(label);
+ return new ReviewersForLabel(label, await this.getState(fileName, "state"));
+ }
+
+ // Writes a ReviewersForLabel object representing the current saved state of which reviewers have reviewed recently.
+ async writeReviewersForLabelState(label: string, newState: any) {
+ var fileName = this.getReviewersForLabelFileName(label);
+ await this.writeState(
+ fileName,
+ "state",
+ new ReviewersForLabel(label, newState)
+ );
+ }
+
+ private async getState(fileName, baseDirectory) {
+ await this.ensureCorrectBranch();
+ fileName = path.join(baseDirectory, fileName);
+ if (!fs.existsSync(fileName)) {
+ return null;
+ }
+ return JSON.parse(fs.readFileSync(fileName, { encoding: "utf-8" }));
+ }
+
+ private async writeState(fileName, baseDirectory, state) {
+ await this.ensureCorrectBranch();
+ fileName = path.join(baseDirectory, fileName);
+ if (!fs.existsSync(baseDirectory)) {
+ fs.mkdirSync(baseDirectory, { recursive: true });
+ }
+ fs.writeFileSync(fileName, JSON.stringify(state, null, 2), {
+ encoding: "utf-8",
+ });
+ await this.commitStateToRepo();
+ }
+
+ private async ensureCorrectBranch() {
+ if (this.switchedBranch) {
+ return;
+ }
+ console.log(
+ "Switching to branch pr-bot-state for reading/storing persistent state between runs"
+ );
+ try {
+ await exec.exec(`git config user.name ${BOT_NAME}`);
+ await exec.exec(`git config user.email ${BOT_NAME}@github.com`);
+ await exec.exec("git config pull.rebase false");
+ await exec.exec("git fetch origin pr-bot-state");
+ await exec.exec("git checkout pr-bot-state");
+ } catch {
+ console.log(
+ "Couldnt find branch pr-bot-state in origin, trying to create it"
+ );
+ try {
+ await exec.exec("git checkout -b pr-bot-state");
+ } catch {
+ console.log("Creating branch failed, trying a simple checkout.");
+ await exec.exec("git checkout pr-bot-state");
+ }
+ }
+ this.switchedBranch = true;
+ }
+
+ private getPrFileName(prNumber) {
Review comment:
It's cleaner for two reasons:
1. `this` usage can be reduced.
2. Pure functions are made very clear, which improves readability and makes reuse very easy in the future.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813184787
##########
File path: scripts/ci/pr-bot/shared/commentStrings.ts
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+export function allChecksPassed(reviewersToNotify: string[]): string {
+ return `All checks have passed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function assignCommitter(committer: string): string {
+ return `R: @${committer} for final approval`;
+}
+
+export function assignReviewer(labelToReviewerMapping: any): string {
+ let commentString =
+ "Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:\n\n";
+
+ for (let label in labelToReviewerMapping) {
+ let reviewer = labelToReviewerMapping[label];
+ if (label == "no-matching-label") {
+ commentString += `R: @${reviewer} added as fallback since no labels match configuration\n`;
+ } else {
+ commentString += `R: @${reviewer} for label ${label}.\n`;
+ }
+ }
+
+ commentString += `
+Available commands:
+- \`stop reviewer notifications\` - opt out of the automated review tooling
+- \`remind me after tests pass\` - tag the comment author after tests pass
+- \`waiting on author\` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)`;
+ return commentString;
+}
+
+export function failingChecksCantAssign(): string {
+ return "Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`\"";
+}
+
+export function someChecksFailing(reviewersToNotify: string[]): string {
+ return `Some checks have failed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function stopNotifications(reason: string): string {
+ return `Stopping reviewer notifications for this pull request: ${reason}`;
+}
+
+export function remindReviewerAfterTestsPass(requester: string): string {
+ return `Ok - I'll remind @${requester} after tests pass`;
+}
+
+export function reviewersAlreadyAssigned(reviewers: string[]): string {
+ let commentString = `Reviewers are already assigned to this pr: `;
+ reviewers.forEach((reviewer) => {
+ commentString += `@${reviewer} `;
+ });
+
+ return commentString;
Review comment:
Perhaps map is cleaner for a one-liner:
```
return `Reviewers are already assigned to this pr: ${reviewers.map(reviewer => '@' + reviewer).join(" ")}`
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813208476
##########
File path: scripts/ci/pr-bot/shared/pr.ts
##########
@@ -0,0 +1,88 @@
+/*
+ * 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 Pr {
+ public commentedAboutFailingChecks: boolean;
+ public reviewersAssignedForLabels: { [key: string]: string };
+ public nextAction: string;
+ public stopReviewerNotifications: boolean;
+ public remindAfterTestsPass: string[];
+
+ constructor(propertyDictionary) {
+ this.commentedAboutFailingChecks = false;
+ this.reviewersAssignedForLabels = {}; // map of label to reviewer
+ this.nextAction = "Author";
+ this.stopReviewerNotifications = false;
+ this.remindAfterTestsPass = []; // List of handles
+
+ if (!propertyDictionary) {
+ return;
+ }
+ if (propertyDictionary) {
+ if ("commentedAboutFailingChecks" in propertyDictionary) {
+ this.commentedAboutFailingChecks =
+ propertyDictionary["commentedAboutFailingChecks"];
+ }
+ if ("reviewersAssignedForLabels" in propertyDictionary) {
+ this.reviewersAssignedForLabels =
+ propertyDictionary["reviewersAssignedForLabels"];
+ }
+ if ("nextAction" in propertyDictionary) {
+ this.nextAction = propertyDictionary["nextAction"];
+ }
+ if ("stopReviewerNotifications" in propertyDictionary) {
+ this.stopReviewerNotifications =
+ propertyDictionary["stopReviewerNotifications"];
+ }
+ if ("remindAfterTestsPass" in propertyDictionary) {
+ this.remindAfterTestsPass = propertyDictionary["remindAfterTestsPass"];
+ }
+ }
+ }
+
+ // Returns a label that the reviewer is assigned for.
+ // If none, returns an empty string
+ getLabelForReviewer(reviewer: string): string {
+ const labels = Object.keys(this.reviewersAssignedForLabels);
+ for (let i = 0; i < labels.length; i++) {
+ let label = labels[i];
+ if (this.reviewersAssignedForLabels[label] == reviewer) {
Review comment:
Triple equals for string matching
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813267954
##########
File path: scripts/ci/pr-bot/shared/commentStrings.ts
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+export function allChecksPassed(reviewersToNotify: string[]): string {
+ return `All checks have passed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function assignCommitter(committer: string): string {
+ return `R: @${committer} for final approval`;
+}
+
+export function assignReviewer(labelToReviewerMapping: any): string {
+ let commentString =
+ "Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:\n\n";
+
+ for (let label in labelToReviewerMapping) {
+ let reviewer = labelToReviewerMapping[label];
+ if (label == "no-matching-label") {
+ commentString += `R: @${reviewer} added as fallback since no labels match configuration\n`;
+ } else {
+ commentString += `R: @${reviewer} for label ${label}.\n`;
+ }
+ }
+
+ commentString += `
+Available commands:
+- \`stop reviewer notifications\` - opt out of the automated review tooling
+- \`remind me after tests pass\` - tag the comment author after tests pass
+- \`waiting on author\` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)`;
+ return commentString;
+}
+
+export function failingChecksCantAssign(): string {
+ return "Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`\"";
+}
+
+export function someChecksFailing(reviewersToNotify: string[]): string {
+ return `Some checks have failed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function stopNotifications(reason: string): string {
+ return `Stopping reviewer notifications for this pull request: ${reason}`;
+}
+
+export function remindReviewerAfterTestsPass(requester: string): string {
+ return `Ok - I'll remind @${requester} after tests pass`;
+}
+
+export function reviewersAlreadyAssigned(reviewers: string[]): string {
+ let commentString = `Reviewers are already assigned to this pr: `;
+ reviewers.forEach((reviewer) => {
+ commentString += `@${reviewer} `;
+ });
+
+ return commentString;
+}
+
+export function noLegalReviewers(): string {
+ return "No reviewers could be found from any of the labels on the pr or in the fallback reviewers list. Check the config file to make sure reviewers are configured";
Review comment:
I updated to PR throughout - I don't think a convention exists in the repo at this point
--
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
[GitHub] [beam] damccorm commented on pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on pull request #16898:
URL: https://github.com/apache/beam/pull/16898#issuecomment-1050233384
R: @lostluck - not sure if there's a better committer, so I'm roping you in (feel free to redirect). Would you mind taking this over the finish line, Jack and Konrad (TS reviewer) have already approved
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813177793
##########
File path: scripts/ci/pr-bot/shared/commentStrings.ts
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+export function allChecksPassed(reviewersToNotify: string[]): string {
+ return `All checks have passed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function assignCommitter(committer: string): string {
+ return `R: @${committer} for final approval`;
+}
+
+export function assignReviewer(labelToReviewerMapping: any): string {
+ let commentString =
+ "Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:\n\n";
+
+ for (let label in labelToReviewerMapping) {
+ let reviewer = labelToReviewerMapping[label];
+ if (label == "no-matching-label") {
Review comment:
Always use triple equals `===` for string matching.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813191489
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
+}
+
+export async function addPrComment(pullNumber: number, body: string) {
+ const githubClient = getGitHubClient();
+ await githubClient.rest.issues.createComment({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ body: body,
Review comment:
`body: body,` => `body,`
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813199389
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
+}
+
+export async function addPrComment(pullNumber: number, body: string) {
+ const githubClient = getGitHubClient();
+ await githubClient.rest.issues.createComment({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ body: body,
+ });
+}
+
+export async function nextActionReviewers(
+ pullNumber: number,
+ existingLabels: any[]
+) {
+ const client = getGitHubClient();
+ let newLabels = removeNextActionLabel(existingLabels);
+ newLabels.push("Next Action: Reviewers");
+ await client.rest.issues.setLabels({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ labels: newLabels,
+ });
+}
+
+export async function nextActionAuthor(
+ pullNumber: number,
+ existingLabels: any[]
+) {
+ const client = getGitHubClient();
+ let newLabels = removeNextActionLabel(existingLabels);
+ newLabels.push("Next Action: Author");
+ await client.rest.issues.setLabels({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ labels: newLabels,
+ });
+}
+
+export async function checkIfCommitter(username: string): Promise<boolean> {
+ const client = getGitHubClient();
+ const permissions = (
+ await client.rest.repos.getCollaboratorPermissionLevel({
+ owner: REPO_OWNER,
+ repo: REPO,
+ username: username,
+ })
+ ).data.permission;
+
+ return permissions == "write" || permissions == "admin";
+}
+
+function removeNextActionLabel(existingLabels: any[]): string[] {
+ let newLabels: string[] = [];
+ existingLabels.forEach((label) => {
+ if (
+ label.name != "Next Action: Reviewers" &&
+ label.name != "Next Action: Author"
+ ) {
+ newLabels.push(label.name);
+ }
+ });
+ return newLabels;
Review comment:
Can be rewritten as:
```
return existingLabels.filter(label => label.name != "..." && label.name != "...").map(label => label.name)
```
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813267514
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
+}
+
+export async function addPrComment(pullNumber: number, body: string) {
+ const githubClient = getGitHubClient();
+ await githubClient.rest.issues.createComment({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ body: body,
+ });
+}
+
+export async function nextActionReviewers(
+ pullNumber: number,
+ existingLabels: any[]
Review comment:
Done - I think there are places in this work where it won't be beneficial to avoid any though (specifically dealing with GitHub payloads which aren't typed on their own). This one is probably fine though.
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
Review comment:
I'd actually vote to keep it as is. The env check is cheap, and this is the only place we ever access it. Not storing it in any persistent state has the advantage of making it very obvious that we're not accidentally leaking it anywhere.
##########
File path: scripts/ci/pr-bot/shared/constants.ts
##########
@@ -0,0 +1,30 @@
+/*
+ * 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 path = require("path");
+
+export const REPO_OWNER = "apache";
+export const REPO = "beam";
+export const PATH_TO_CONFIG_FILE = path.join(
+ __dirname,
+ "../../../../../.github/REVIEWERS.yml"
Review comment:
I didn't want to do that since I don't think there's a consistent way to get the repo root from both GitHub actions and locally, and I'd like this constant to be valid in both cases.
--
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
[GitHub] [beam] lostluck merged pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
lostluck merged pull request #16898:
URL: https://github.com/apache/beam/pull/16898
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r814208641
##########
File path: scripts/ci/pr-bot/shared/reviewerConfig.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 yaml = require("js-yaml");
+const fs = require("fs");
+import { Label } from "./githubUtils";
+
+export class ReviewerConfig {
+ private config: any;
+ constructor(pathToConfigFile) {
+ this.config = yaml.load(
+ fs.readFileSync(pathToConfigFile, { encoding: "utf-8" })
+ );
+ }
+
+ // Given a list of labels and an exclusion list of reviewers not to include (e.g. the author)
+ // returns all possible reviewers for each label
+ getReviewersForLabels(
+ labels: Label[],
+ exclusionList: string[]
+ ): { [key: string]: string[] } {
+ let reviewersFound = false;
+ let labelToReviewerMapping = {};
+ labels.forEach((label) => {
+ let reviewers = this.getReviewersForLabel(label.name, exclusionList);
+ if (reviewers.length > 0) {
+ labelToReviewerMapping[label.name] = reviewers;
+ reviewersFound = true;
+ }
+ });
+ if (!reviewersFound) {
+ const fallbackReviewers = this.getFallbackReviewers(exclusionList);
+ if (fallbackReviewers.length > 0) {
+ labelToReviewerMapping["no-matching-label"] =
+ this.getFallbackReviewers(exclusionList);
+ }
+ }
+ return labelToReviewerMapping;
+ }
+
+ // Get possible reviewers excluding the author.
+ getReviewersForLabel(label: string, exclusionList: string[]): string[] {
+ var labelObjects = this.config.labels;
+ for (var i = 0; i < labelObjects.length; i++) {
+ var labelObject = labelObjects[i];
+ if (labelObject.name.toLowerCase() === label.toLowerCase()) {
+ return this.excludeFromReviewers(labelObject.reviewers, exclusionList);
+ }
+ }
+ return [];
Review comment:
Perhaps this block could be cleaner using [find](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find).
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813190972
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
+}
+
+export async function addPrComment(pullNumber: number, body: string) {
+ const githubClient = getGitHubClient();
Review comment:
Storing the githubClient in a variable has little value. Please remove this line.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813201977
##########
File path: scripts/ci/pr-bot/shared/commentStrings.ts
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+export function allChecksPassed(reviewersToNotify: string[]): string {
Review comment:
Exported functions typically must have comments, most linters will complain about this. That being said, I'm not sure how strict we should be on this code.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813185392
##########
File path: scripts/ci/pr-bot/shared/commentStrings.ts
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+export function allChecksPassed(reviewersToNotify: string[]): string {
+ return `All checks have passed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function assignCommitter(committer: string): string {
+ return `R: @${committer} for final approval`;
+}
+
+export function assignReviewer(labelToReviewerMapping: any): string {
+ let commentString =
+ "Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:\n\n";
+
+ for (let label in labelToReviewerMapping) {
+ let reviewer = labelToReviewerMapping[label];
+ if (label == "no-matching-label") {
+ commentString += `R: @${reviewer} added as fallback since no labels match configuration\n`;
+ } else {
+ commentString += `R: @${reviewer} for label ${label}.\n`;
+ }
+ }
+
+ commentString += `
+Available commands:
+- \`stop reviewer notifications\` - opt out of the automated review tooling
+- \`remind me after tests pass\` - tag the comment author after tests pass
+- \`waiting on author\` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)`;
+ return commentString;
+}
+
+export function failingChecksCantAssign(): string {
+ return "Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment `assign set of reviewers`\"";
+}
+
+export function someChecksFailing(reviewersToNotify: string[]): string {
+ return `Some checks have failed: @${reviewersToNotify.join(" ")}`;
+}
+
+export function stopNotifications(reason: string): string {
+ return `Stopping reviewer notifications for this pull request: ${reason}`;
+}
+
+export function remindReviewerAfterTestsPass(requester: string): string {
+ return `Ok - I'll remind @${requester} after tests pass`;
+}
+
+export function reviewersAlreadyAssigned(reviewers: string[]): string {
+ let commentString = `Reviewers are already assigned to this pr: `;
+ reviewers.forEach((reviewer) => {
+ commentString += `@${reviewer} `;
+ });
+
+ return commentString;
+}
+
+export function noLegalReviewers(): string {
+ return "No reviewers could be found from any of the labels on the pr or in the fallback reviewers list. Check the config file to make sure reviewers are configured";
Review comment:
`pr` is an initialism. Perhaps capitalize the letters => `PR`. That being said, please stay consistent with the way it is done throughout the repo.
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813267084
##########
File path: scripts/ci/pr-bot/shared/persistentState.ts
##########
@@ -0,0 +1,123 @@
+/*
+ * 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 exec = require("@actions/exec");
+const fs = require("fs");
+const path = require("path");
+const { Pr } = require("./pr");
+const { ReviewersForLabel } = require("./reviewersForLabel");
+const { BOT_NAME } = require("./constants");
+
+export class PersistentState {
+ private switchedBranch: boolean;
+
+ constructor() {
+ this.switchedBranch = false;
+ }
+
+ // Returns a Pr object representing the current saved state of the pr.
+ async getPrState(prNumber: number): Promise<any> {
+ var fileName = this.getPrFileName(prNumber);
+ return new Pr(await this.getState(fileName, "state/pr-state"));
+ }
+
+ // Writes a Pr object representing the current saved state of the pr to persistent storage.
+ async writePrState(prNumber: number, newState: any) {
+ var fileName = this.getPrFileName(prNumber);
+ await this.writeState(fileName, "state/pr-state", new Pr(newState));
+ }
+
+ // Returns a ReviewersForLabel object representing the current saved state of which reviewers have reviewed recently.
+ async getReviewersForLabelState(label: string): Promise<any> {
+ var fileName = this.getReviewersForLabelFileName(label);
+ return new ReviewersForLabel(label, await this.getState(fileName, "state"));
+ }
+
+ // Writes a ReviewersForLabel object representing the current saved state of which reviewers have reviewed recently.
+ async writeReviewersForLabelState(label: string, newState: any) {
+ var fileName = this.getReviewersForLabelFileName(label);
+ await this.writeState(
+ fileName,
+ "state",
+ new ReviewersForLabel(label, newState)
+ );
+ }
+
+ private async getState(fileName, baseDirectory) {
+ await this.ensureCorrectBranch();
+ fileName = path.join(baseDirectory, fileName);
+ if (!fs.existsSync(fileName)) {
+ return null;
+ }
+ return JSON.parse(fs.readFileSync(fileName, { encoding: "utf-8" }));
+ }
+
+ private async writeState(fileName, baseDirectory, state) {
+ await this.ensureCorrectBranch();
+ fileName = path.join(baseDirectory, fileName);
+ if (!fs.existsSync(baseDirectory)) {
+ fs.mkdirSync(baseDirectory, { recursive: true });
+ }
+ fs.writeFileSync(fileName, JSON.stringify(state, null, 2), {
+ encoding: "utf-8",
+ });
+ await this.commitStateToRepo();
+ }
+
+ private async ensureCorrectBranch() {
+ if (this.switchedBranch) {
+ return;
+ }
+ console.log(
+ "Switching to branch pr-bot-state for reading/storing persistent state between runs"
+ );
+ try {
+ await exec.exec(`git config user.name ${BOT_NAME}`);
+ await exec.exec(`git config user.email ${BOT_NAME}@github.com`);
+ await exec.exec("git config pull.rebase false");
+ await exec.exec("git fetch origin pr-bot-state");
+ await exec.exec("git checkout pr-bot-state");
+ } catch {
+ console.log(
+ "Couldnt find branch pr-bot-state in origin, trying to create it"
+ );
+ try {
+ await exec.exec("git checkout -b pr-bot-state");
+ } catch {
+ console.log("Creating branch failed, trying a simple checkout.");
+ await exec.exec("git checkout pr-bot-state");
+ }
+ }
+ this.switchedBranch = true;
+ }
+
+ private getPrFileName(prNumber) {
Review comment:
Is there a reason that's actually cleaner? I don't particularly like separating this (and the other private functions) from the class because they are very coupled to the class' protocol for storing state in the filesystem. IMO they are part of the object's unique way of storing/accessing state and thus belong as part of the object for readability.
##########
File path: scripts/ci/pr-bot/shared/persistentState.ts
##########
@@ -0,0 +1,123 @@
+/*
+ * 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 exec = require("@actions/exec");
+const fs = require("fs");
+const path = require("path");
+const { Pr } = require("./pr");
+const { ReviewersForLabel } = require("./reviewersForLabel");
+const { BOT_NAME } = require("./constants");
+
+export class PersistentState {
+ private switchedBranch: boolean;
+
+ constructor() {
+ this.switchedBranch = false;
+ }
+
+ // Returns a Pr object representing the current saved state of the pr.
+ async getPrState(prNumber: number): Promise<any> {
Review comment:
I think I can just use `typeof Pr`
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r814211130
##########
File path: scripts/ci/pr-bot/shared/reviewerConfig.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 yaml = require("js-yaml");
+const fs = require("fs");
+import { Label } from "./githubUtils";
+
+export class ReviewerConfig {
+ private config: any;
+ constructor(pathToConfigFile) {
+ this.config = yaml.load(
+ fs.readFileSync(pathToConfigFile, { encoding: "utf-8" })
+ );
+ }
+
+ // Given a list of labels and an exclusion list of reviewers not to include (e.g. the author)
+ // returns all possible reviewers for each label
+ getReviewersForLabels(
+ labels: Label[],
+ exclusionList: string[]
+ ): { [key: string]: string[] } {
+ let reviewersFound = false;
+ let labelToReviewerMapping = {};
+ labels.forEach((label) => {
+ let reviewers = this.getReviewersForLabel(label.name, exclusionList);
+ if (reviewers.length > 0) {
+ labelToReviewerMapping[label.name] = reviewers;
+ reviewersFound = true;
+ }
+ });
+ if (!reviewersFound) {
+ const fallbackReviewers = this.getFallbackReviewers(exclusionList);
+ if (fallbackReviewers.length > 0) {
+ labelToReviewerMapping["no-matching-label"] =
+ this.getFallbackReviewers(exclusionList);
+ }
+ }
+ return labelToReviewerMapping;
+ }
+
+ // Get possible reviewers excluding the author.
+ getReviewersForLabel(label: string, exclusionList: string[]): string[] {
+ var labelObjects = this.config.labels;
+ for (var i = 0; i < labelObjects.length; i++) {
+ var labelObject = labelObjects[i];
+ if (labelObject.name.toLowerCase() === label.toLowerCase()) {
+ return this.excludeFromReviewers(labelObject.reviewers, exclusionList);
+ }
+ }
+ return [];
+ }
+
+ getExclusionListForLabel(label: string): string[] {
+ var labelObjects = this.config.labels;
+ for (var i = 0; i < labelObjects.length; i++) {
+ var labelObject = labelObjects[i];
+ if (labelObject.name.toLowerCase() === label.toLowerCase()) {
+ return labelObject.exclusionList;
+ }
+ }
+ return [];
+ }
+
+ // Get fallback reviewers excluding the author.
+ getFallbackReviewers(exclusionList: string[]): string[] {
+ return this.excludeFromReviewers(
+ this.config.fallbackReviewers,
+ exclusionList
+ );
+ }
+
+ private excludeFromReviewers(
+ reviewers: string[],
+ exclusionList: string[]
+ ): string[] {
+ if (!exclusionList) {
+ return reviewers;
+ }
+ exclusionList.forEach((reviewer) => {
+ const reviewerIndex = reviewers.indexOf(reviewer);
+ if (reviewerIndex > -1) {
+ reviewers.splice(reviewerIndex, 1);
+ }
+ });
+ return reviewers;
Review comment:
Perhaps this is cleaner:
```
return reviewers.filter(reviewer => exclusionList.has(reviewer))
```
##########
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:
Isn't this async? I think an await is missing here.
##########
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:
`any` => `unknown`
##########
File path: scripts/ci/pr-bot/test/reviewerConfigTest.ts
##########
@@ -0,0 +1,218 @@
+/*
+ * 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.
+ */
+
+var assert = require("assert");
+var fs = require("fs");
+const { ReviewerConfig } = require("../shared/reviewerConfig");
+const configPath = "test-config.yml";
+const configContents = `labels:
+- name: "Go"
+ reviewers: ["testReviewer1", "testReviewer2"]
+ exclusionList: ["testReviewer3"] # These users will never be suggested as reviewers
+# I don't know the other areas well enough to assess who the normal committers/contributors who might want to be reviewers are
+- name: "Java"
+ reviewers: ["testReviewer3", "testReviewer2"]
+ exclusionList: [] # These users will never be suggested as reviewers
+- name: "Python"
+ reviewers: ["testReviewer4"]
+ exclusionList: [] # These users will never be suggested as reviewers
+fallbackReviewers: ["testReviewer5", "testReviewer1", "testReviewer3"] # List of committers to use when no label matches
+`;
+describe("ReviewerConfig", function () {
+ before(function () {
+ if (fs.existsSync(configPath)) {
+ fs.rmSync(configPath);
+ }
+ fs.writeFileSync(configPath, configContents);
+ });
+
+ after(function () {
+ fs.rmSync(configPath);
+ });
+
+ describe("getReviewersForLabels()", function () {
+ it("should return all reviewers configured for all labels", function () {
+ const config = new ReviewerConfig(configPath);
+ const reviewersForLabels = config.getReviewersForLabels(
+ [{ name: "Go" }, { name: "Java" }],
+ []
+ );
+ assert(
+ reviewersForLabels["Go"].indexOf("testReviewer1") > -1,
Review comment:
Using `find` instead of `indexOf` would be cleaner because it returns null which is false.
##########
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:
`labels?.length > 0`
##########
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) {
Review comment:
Triple equals for strict number comparison.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813195091
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
+}
+
+export async function addPrComment(pullNumber: number, body: string) {
+ const githubClient = getGitHubClient();
+ await githubClient.rest.issues.createComment({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ body: body,
+ });
+}
+
+export async function nextActionReviewers(
+ pullNumber: number,
+ existingLabels: any[]
Review comment:
Avoid any type. Add a definition to the top of the file if needed:
```
interface Label {
name: string;
}
```
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813189299
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
Review comment:
Preferably this token could be stored as a constant somewhere after the env has initialized.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r814209009
##########
File path: scripts/ci/pr-bot/shared/reviewerConfig.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 yaml = require("js-yaml");
+const fs = require("fs");
+import { Label } from "./githubUtils";
+
+export class ReviewerConfig {
+ private config: any;
+ constructor(pathToConfigFile) {
+ this.config = yaml.load(
+ fs.readFileSync(pathToConfigFile, { encoding: "utf-8" })
+ );
+ }
+
+ // Given a list of labels and an exclusion list of reviewers not to include (e.g. the author)
+ // returns all possible reviewers for each label
+ getReviewersForLabels(
+ labels: Label[],
+ exclusionList: string[]
+ ): { [key: string]: string[] } {
+ let reviewersFound = false;
+ let labelToReviewerMapping = {};
+ labels.forEach((label) => {
+ let reviewers = this.getReviewersForLabel(label.name, exclusionList);
+ if (reviewers.length > 0) {
+ labelToReviewerMapping[label.name] = reviewers;
+ reviewersFound = true;
+ }
+ });
+ if (!reviewersFound) {
+ const fallbackReviewers = this.getFallbackReviewers(exclusionList);
+ if (fallbackReviewers.length > 0) {
+ labelToReviewerMapping["no-matching-label"] =
+ this.getFallbackReviewers(exclusionList);
+ }
+ }
+ return labelToReviewerMapping;
+ }
+
+ // Get possible reviewers excluding the author.
+ getReviewersForLabel(label: string, exclusionList: string[]): string[] {
+ var labelObjects = this.config.labels;
+ for (var i = 0; i < labelObjects.length; i++) {
+ var labelObject = labelObjects[i];
+ if (labelObject.name.toLowerCase() === label.toLowerCase()) {
+ return this.excludeFromReviewers(labelObject.reviewers, exclusionList);
+ }
+ }
+ return [];
+ }
+
+ getExclusionListForLabel(label: string): string[] {
+ var labelObjects = this.config.labels;
+ for (var i = 0; i < labelObjects.length; i++) {
+ var labelObject = labelObjects[i];
+ if (labelObject.name.toLowerCase() === label.toLowerCase()) {
+ return labelObject.exclusionList;
+ }
+ }
+ return [];
Review comment:
Definitely using [find](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find) will be cleaner here.
--
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
[GitHub] [beam] aaltay commented on pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #16898:
URL: https://github.com/apache/beam/pull/16898#issuecomment-1045277146
If he has time @konradjanica would be a good reviewer for typescript.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813197243
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
+}
+
+export async function addPrComment(pullNumber: number, body: string) {
+ const githubClient = getGitHubClient();
+ await githubClient.rest.issues.createComment({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ body: body,
+ });
+}
+
+export async function nextActionReviewers(
+ pullNumber: number,
+ existingLabels: any[]
+) {
+ const client = getGitHubClient();
+ let newLabels = removeNextActionLabel(existingLabels);
+ newLabels.push("Next Action: Reviewers");
+ await client.rest.issues.setLabels({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ labels: newLabels,
+ });
+}
+
+export async function nextActionAuthor(
+ pullNumber: number,
+ existingLabels: any[]
+) {
+ const client = getGitHubClient();
+ let newLabels = removeNextActionLabel(existingLabels);
+ newLabels.push("Next Action: Author");
+ await client.rest.issues.setLabels({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ labels: newLabels,
+ });
+}
+
+export async function checkIfCommitter(username: string): Promise<boolean> {
+ const client = getGitHubClient();
+ const permissions = (
Review comment:
I think it would be more readable to store the response and then reference the permission from the response. It would be the same amount of lines too.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813197479
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
+}
+
+export async function addPrComment(pullNumber: number, body: string) {
+ const githubClient = getGitHubClient();
+ await githubClient.rest.issues.createComment({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ body: body,
+ });
+}
+
+export async function nextActionReviewers(
+ pullNumber: number,
+ existingLabels: any[]
+) {
+ const client = getGitHubClient();
+ let newLabels = removeNextActionLabel(existingLabels);
+ newLabels.push("Next Action: Reviewers");
+ await client.rest.issues.setLabels({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ labels: newLabels,
+ });
+}
+
+export async function nextActionAuthor(
+ pullNumber: number,
+ existingLabels: any[]
+) {
+ const client = getGitHubClient();
+ let newLabels = removeNextActionLabel(existingLabels);
+ newLabels.push("Next Action: Author");
+ await client.rest.issues.setLabels({
+ owner: REPO_OWNER,
+ repo: REPO,
+ issue_number: pullNumber,
+ labels: newLabels,
+ });
+}
+
+export async function checkIfCommitter(username: string): Promise<boolean> {
+ const client = getGitHubClient();
+ const permissions = (
+ await client.rest.repos.getCollaboratorPermissionLevel({
+ owner: REPO_OWNER,
+ repo: REPO,
+ username: username,
+ })
+ ).data.permission;
+
+ return permissions == "write" || permissions == "admin";
Review comment:
Triple equals for string matching, e.g. `===`
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813188566
##########
File path: scripts/ci/pr-bot/shared/githubUtils.ts
##########
@@ -0,0 +1,96 @@
+/*
+ * 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 { Octokit } = require("@octokit/rest");
+const { REPO_OWNER, REPO } = require("./constants");
+
+export function getGitHubClient() {
+ let githubToken = process.env["GITHUB_TOKEN"];
+ if (!githubToken) {
+ throw new Error(
+ "No github token provided - process.env['GITHUB_TOKEN'] must be set."
+ );
+ }
+ return new Octokit({ auth: githubToken });
Review comment:
Perhaps rename `githubToken` => `auth` so that the return is a bit cleaner: `return new Octokit({ auth });`
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813206009
##########
File path: scripts/ci/pr-bot/shared/persistentState.ts
##########
@@ -0,0 +1,123 @@
+/*
+ * 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 exec = require("@actions/exec");
+const fs = require("fs");
+const path = require("path");
+const { Pr } = require("./pr");
+const { ReviewersForLabel } = require("./reviewersForLabel");
+const { BOT_NAME } = require("./constants");
+
+export class PersistentState {
+ private switchedBranch: boolean;
+
+ constructor() {
+ this.switchedBranch = false;
+ }
+
+ // Returns a Pr object representing the current saved state of the pr.
+ async getPrState(prNumber: number): Promise<any> {
+ var fileName = this.getPrFileName(prNumber);
+ return new Pr(await this.getState(fileName, "state/pr-state"));
+ }
+
+ // Writes a Pr object representing the current saved state of the pr to persistent storage.
+ async writePrState(prNumber: number, newState: any) {
+ var fileName = this.getPrFileName(prNumber);
+ await this.writeState(fileName, "state/pr-state", new Pr(newState));
+ }
+
+ // Returns a ReviewersForLabel object representing the current saved state of which reviewers have reviewed recently.
+ async getReviewersForLabelState(label: string): Promise<any> {
+ var fileName = this.getReviewersForLabelFileName(label);
+ return new ReviewersForLabel(label, await this.getState(fileName, "state"));
+ }
+
+ // Writes a ReviewersForLabel object representing the current saved state of which reviewers have reviewed recently.
+ async writeReviewersForLabelState(label: string, newState: any) {
+ var fileName = this.getReviewersForLabelFileName(label);
+ await this.writeState(
+ fileName,
+ "state",
+ new ReviewersForLabel(label, newState)
+ );
+ }
+
+ private async getState(fileName, baseDirectory) {
+ await this.ensureCorrectBranch();
+ fileName = path.join(baseDirectory, fileName);
+ if (!fs.existsSync(fileName)) {
+ return null;
+ }
+ return JSON.parse(fs.readFileSync(fileName, { encoding: "utf-8" }));
+ }
+
+ private async writeState(fileName, baseDirectory, state) {
+ await this.ensureCorrectBranch();
+ fileName = path.join(baseDirectory, fileName);
+ if (!fs.existsSync(baseDirectory)) {
+ fs.mkdirSync(baseDirectory, { recursive: true });
+ }
+ fs.writeFileSync(fileName, JSON.stringify(state, null, 2), {
+ encoding: "utf-8",
+ });
+ await this.commitStateToRepo();
+ }
+
+ private async ensureCorrectBranch() {
+ if (this.switchedBranch) {
+ return;
+ }
+ console.log(
+ "Switching to branch pr-bot-state for reading/storing persistent state between runs"
+ );
+ try {
+ await exec.exec(`git config user.name ${BOT_NAME}`);
+ await exec.exec(`git config user.email ${BOT_NAME}@github.com`);
+ await exec.exec("git config pull.rebase false");
+ await exec.exec("git fetch origin pr-bot-state");
+ await exec.exec("git checkout pr-bot-state");
+ } catch {
+ console.log(
+ "Couldnt find branch pr-bot-state in origin, trying to create it"
+ );
+ try {
+ await exec.exec("git checkout -b pr-bot-state");
+ } catch {
+ console.log("Creating branch failed, trying a simple checkout.");
+ await exec.exec("git checkout pr-bot-state");
+ }
+ }
+ this.switchedBranch = true;
+ }
+
+ private getPrFileName(prNumber) {
Review comment:
Avoid member functions unless the classes state should be referenced. The cleanest option is to make this into an unexported helper at the top or bottom of the file. That way you can avoid `this.getPrFileName` everywhere.
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813266916
##########
File path: scripts/ci/pr-bot/shared/pr.ts
##########
@@ -0,0 +1,88 @@
+/*
+ * 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 Pr {
+ public commentedAboutFailingChecks: boolean;
+ public reviewersAssignedForLabels: { [key: string]: string };
+ public nextAction: string;
+ public stopReviewerNotifications: boolean;
+ public remindAfterTestsPass: string[];
+
+ constructor(propertyDictionary) {
+ this.commentedAboutFailingChecks = false;
+ this.reviewersAssignedForLabels = {}; // map of label to reviewer
+ this.nextAction = "Author";
+ this.stopReviewerNotifications = false;
+ this.remindAfterTestsPass = []; // List of handles
+
+ if (!propertyDictionary) {
+ return;
+ }
+ if (propertyDictionary) {
+ if ("commentedAboutFailingChecks" in propertyDictionary) {
+ this.commentedAboutFailingChecks =
+ propertyDictionary["commentedAboutFailingChecks"];
+ }
+ if ("reviewersAssignedForLabels" in propertyDictionary) {
+ this.reviewersAssignedForLabels =
+ propertyDictionary["reviewersAssignedForLabels"];
+ }
+ if ("nextAction" in propertyDictionary) {
+ this.nextAction = propertyDictionary["nextAction"];
+ }
+ if ("stopReviewerNotifications" in propertyDictionary) {
+ this.stopReviewerNotifications =
+ propertyDictionary["stopReviewerNotifications"];
+ }
+ if ("remindAfterTestsPass" in propertyDictionary) {
+ this.remindAfterTestsPass = propertyDictionary["remindAfterTestsPass"];
+ }
+ }
+ }
+
+ // Returns a label that the reviewer is assigned for.
+ // If none, returns an empty string
+ getLabelForReviewer(reviewer: string): string {
+ const labels = Object.keys(this.reviewersAssignedForLabels);
+ for (let i = 0; i < labels.length; i++) {
+ let label = labels[i];
+ if (this.reviewersAssignedForLabels[label] == reviewer) {
+ return label;
+ }
+ }
+
+ return "";
+ }
+
+ // Returns whether any of the assigned reviewers are committers
+ async isAnyAssignedReviewerCommitter(): Promise<boolean> {
+ const labels = Object.keys(this.reviewersAssignedForLabels);
+ for (let i = 0; i < labels.length; i++) {
+ let label = labels[i];
+ if (
+ await github.checkIfCommitter(this.reviewersAssignedForLabels[label])
Review comment:
There isn't - I would expect relatively few calls here though - this represents all reviewers assigned to the pr, so there should never be more than a few (definitely <=5, more likely 1-2).
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813174112
##########
File path: scripts/ci/pr-bot/README.md
##########
@@ -0,0 +1,45 @@
+<!--
+ 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.
+-->
+
+# Pr Bot
+
+This directory holds all the code (except for Actions Workflows) for our pr bot designed to improve the pr experience.
Review comment:
`pr` is an initialism. Perhaps capitalize the letters => `PR`. That being said, please stay consistent with the way it is done throughout the repo.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813211026
##########
File path: scripts/ci/pr-bot/shared/pr.ts
##########
@@ -0,0 +1,88 @@
+/*
+ * 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 Pr {
+ public commentedAboutFailingChecks: boolean;
+ public reviewersAssignedForLabels: { [key: string]: string };
+ public nextAction: string;
+ public stopReviewerNotifications: boolean;
+ public remindAfterTestsPass: string[];
+
+ constructor(propertyDictionary) {
+ this.commentedAboutFailingChecks = false;
+ this.reviewersAssignedForLabels = {}; // map of label to reviewer
+ this.nextAction = "Author";
+ this.stopReviewerNotifications = false;
+ this.remindAfterTestsPass = []; // List of handles
+
+ if (!propertyDictionary) {
+ return;
+ }
+ if (propertyDictionary) {
+ if ("commentedAboutFailingChecks" in propertyDictionary) {
+ this.commentedAboutFailingChecks =
+ propertyDictionary["commentedAboutFailingChecks"];
+ }
+ if ("reviewersAssignedForLabels" in propertyDictionary) {
+ this.reviewersAssignedForLabels =
+ propertyDictionary["reviewersAssignedForLabels"];
+ }
+ if ("nextAction" in propertyDictionary) {
+ this.nextAction = propertyDictionary["nextAction"];
+ }
+ if ("stopReviewerNotifications" in propertyDictionary) {
+ this.stopReviewerNotifications =
+ propertyDictionary["stopReviewerNotifications"];
+ }
+ if ("remindAfterTestsPass" in propertyDictionary) {
+ this.remindAfterTestsPass = propertyDictionary["remindAfterTestsPass"];
+ }
+ }
+ }
+
+ // Returns a label that the reviewer is assigned for.
+ // If none, returns an empty string
+ getLabelForReviewer(reviewer: string): string {
+ const labels = Object.keys(this.reviewersAssignedForLabels);
+ for (let i = 0; i < labels.length; i++) {
+ let label = labels[i];
+ if (this.reviewersAssignedForLabels[label] == reviewer) {
+ return label;
+ }
+ }
+
+ return "";
+ }
+
+ // Returns whether any of the assigned reviewers are committers
+ async isAnyAssignedReviewerCommitter(): Promise<boolean> {
+ const labels = Object.keys(this.reviewersAssignedForLabels);
+ for (let i = 0; i < labels.length; i++) {
+ let label = labels[i];
+ if (
+ await github.checkIfCommitter(this.reviewersAssignedForLabels[label])
Review comment:
This call isn't batched so it will be very slow. Is there a batched version of the API?
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813209631
##########
File path: scripts/ci/pr-bot/shared/pr.ts
##########
@@ -0,0 +1,88 @@
+/*
+ * 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 Pr {
+ public commentedAboutFailingChecks: boolean;
+ public reviewersAssignedForLabels: { [key: string]: string };
+ public nextAction: string;
+ public stopReviewerNotifications: boolean;
+ public remindAfterTestsPass: string[];
+
+ constructor(propertyDictionary) {
+ this.commentedAboutFailingChecks = false;
+ this.reviewersAssignedForLabels = {}; // map of label to reviewer
+ this.nextAction = "Author";
+ this.stopReviewerNotifications = false;
+ this.remindAfterTestsPass = []; // List of handles
+
+ if (!propertyDictionary) {
+ return;
+ }
+ if (propertyDictionary) {
+ if ("commentedAboutFailingChecks" in propertyDictionary) {
+ this.commentedAboutFailingChecks =
+ propertyDictionary["commentedAboutFailingChecks"];
+ }
+ if ("reviewersAssignedForLabels" in propertyDictionary) {
+ this.reviewersAssignedForLabels =
+ propertyDictionary["reviewersAssignedForLabels"];
+ }
+ if ("nextAction" in propertyDictionary) {
+ this.nextAction = propertyDictionary["nextAction"];
+ }
+ if ("stopReviewerNotifications" in propertyDictionary) {
+ this.stopReviewerNotifications =
+ propertyDictionary["stopReviewerNotifications"];
+ }
+ if ("remindAfterTestsPass" in propertyDictionary) {
+ this.remindAfterTestsPass = propertyDictionary["remindAfterTestsPass"];
+ }
+ }
+ }
+
+ // Returns a label that the reviewer is assigned for.
+ // If none, returns an empty string
+ getLabelForReviewer(reviewer: string): string {
+ const labels = Object.keys(this.reviewersAssignedForLabels);
+ for (let i = 0; i < labels.length; i++) {
+ let label = labels[i];
+ if (this.reviewersAssignedForLabels[label] == reviewer) {
+ return label;
+ }
+ }
+
+ return "";
+ }
+
+ // Returns whether any of the assigned reviewers are committers
+ async isAnyAssignedReviewerCommitter(): Promise<boolean> {
+ const labels = Object.keys(this.reviewersAssignedForLabels);
+ for (let i = 0; i < labels.length; i++) {
+ let label = labels[i];
Review comment:
Save the line by not assigning this variable.
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813268227
##########
File path: scripts/ci/pr-bot/README.md
##########
@@ -0,0 +1,45 @@
+<!--
+ 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.
+-->
+
+# Pr Bot
+
+This directory holds all the code (except for Actions Workflows) for our pr bot designed to improve the pr experience.
+For a list of commands to use when interacting with the bot, see [Commands.md](./Commands.md).
+For a design doc explaining the design and implementation, see [Automate Reviewer Assignment](https://docs.google.com/document/d/1FhRPRD6VXkYlLAPhNfZB7y2Yese2FCWBzjx67d3TjBo/edit#)
+
+## Build/Test
+
+To build, run:
+
+```
+npm install
+npm run build
+```
+
+To run the tests:
+
+```
+npm test
+```
+
+Before checking in code, run prettier on it:
+
+```
+npm run format
Review comment:
I agree we want some enforcement, I opted to scope that out of this PR though
##########
File path: scripts/ci/pr-bot/README.md
##########
@@ -0,0 +1,45 @@
+<!--
+ 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.
+-->
+
+# Pr Bot
+
+This directory holds all the code (except for Actions Workflows) for our pr bot designed to improve the pr experience.
Review comment:
Sure, updated
--
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
[GitHub] [beam] damccorm commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
damccorm commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813267240
##########
File path: scripts/ci/pr-bot/shared/commentStrings.ts
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+export function allChecksPassed(reviewersToNotify: string[]): string {
Review comment:
None of this is being published to a package (its all internal code), so I'd vote we remain pretty loose with that. There's functionally not a difference between exported and non-exported files other than maintaining good repo structure.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813202569
##########
File path: scripts/ci/pr-bot/shared/persistentState.ts
##########
@@ -0,0 +1,123 @@
+/*
+ * 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 exec = require("@actions/exec");
+const fs = require("fs");
+const path = require("path");
+const { Pr } = require("./pr");
+const { ReviewersForLabel } = require("./reviewersForLabel");
+const { BOT_NAME } = require("./constants");
+
+export class PersistentState {
+ private switchedBranch: boolean;
Review comment:
Initialize the variable here and drop the typing. That way you can remove the constructor.
--
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
[GitHub] [beam] KonradJanica commented on a change in pull request #16898: [BEAM-13925] Add most of the supporting files for the pr management automation
Posted by GitBox <gi...@apache.org>.
KonradJanica commented on a change in pull request #16898:
URL: https://github.com/apache/beam/pull/16898#discussion_r813186839
##########
File path: scripts/ci/pr-bot/shared/constants.ts
##########
@@ -0,0 +1,30 @@
+/*
+ * 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 path = require("path");
+
+export const REPO_OWNER = "apache";
+export const REPO = "beam";
+export const PATH_TO_CONFIG_FILE = path.join(
+ __dirname,
+ "../../../../../.github/REVIEWERS.yml"
Review comment:
This is very brittle and difficult to read. Could we use a path from the root instead?
--
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