-
Notifications
You must be signed in to change notification settings - Fork 7
Community automations updates: bot and community review #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| name: Handle pull request events | ||
| on: | ||
| pull_request_target: | ||
| types: [review_requested,labeled] | ||
| jobs: | ||
| call-workflow: | ||
| name: Call shared workflow | ||
| uses: learningequality/.github/.github/workflows/pull-request-target.yml@main | ||
| secrets: | ||
| LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} | ||
| LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| name: Handle label added on community pull request | ||
| on: | ||
| workflow_call: | ||
| secrets: | ||
| LE_BOT_APP_ID: | ||
| description: 'GitHub App ID for authentication' | ||
| required: true | ||
| LE_BOT_PRIVATE_KEY: | ||
| description: 'GitHub App Private Key for authentication' | ||
| required: true | ||
| jobs: | ||
| check-if-contributor: | ||
| name: Check if author is contributor | ||
| uses: learningequality/.github/.github/workflows/is-contributor.yml@main | ||
| secrets: | ||
| LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} | ||
| LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} | ||
| with: | ||
| username: ${{ github.event.pull_request.user.login }} | ||
| author_association: ${{ github.event.pull_request.author_association }} | ||
| handle-label: | ||
| name: Handle label | ||
| needs: [check-if-contributor] | ||
| if: ${{ needs.check-if-contributor.outputs.is_contributor == 'true' }} | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Generate App Token | ||
| id: generate-token | ||
| uses: actions/create-github-app-token@v2 | ||
| with: | ||
| app-id: ${{ secrets.LE_BOT_APP_ID }} | ||
| private-key: ${{ secrets.LE_BOT_PRIVATE_KEY }} | ||
| - name: Checkout .github repository | ||
| uses: actions/checkout@v4 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Old version of the action. |
||
| with: | ||
| repository: learningequality/.github | ||
| ref: main | ||
| token: ${{ steps.generate-token.outputs.token }} | ||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ibid |
||
| with: | ||
| node-version: '20' | ||
| - name: Install dependencies | ||
| run: yarn install --frozen-lockfile | ||
| - name: Run script | ||
| id: script | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| github-token: ${{ steps.generate-token.outputs.token }} | ||
| script: | | ||
| const script = require('./scripts/pull-request-label.js'); | ||
| return await script({ github, context, core }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| name: Handle pull request events | ||
| on: | ||
| workflow_call: | ||
| secrets: | ||
| LE_BOT_APP_ID: | ||
| description: 'GitHub App ID for authentication' | ||
| required: true | ||
| LE_BOT_PRIVATE_KEY: | ||
| description: 'GitHub App Private Key for authentication' | ||
| required: true | ||
| jobs: | ||
| review-requested: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this structure - I imagine we could make another layer of the workflows that triages between issue and pull request events, so we only ever have to deal with a single point of entry.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, with a small limitation #50 (comment) (you reviewed faster than I wrote it :) |
||
| name: Handle review requested | ||
| if: ${{ github.event.action == 'review_requested' }} | ||
| uses: learningequality/.github/.github/workflows/review-requested.yml@main | ||
| secrets: | ||
| LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} | ||
| LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} | ||
| pull-request-label: | ||
| name: Handle pull request label | ||
| if: ${{ github.event.action == 'labeled' }} | ||
| uses: learningequality/.github/.github/workflows/pull-request-label.yml@main | ||
| secrets: | ||
| LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} | ||
| LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| name: Handle review requested on community pull request | ||
| on: | ||
| workflow_call: | ||
| secrets: | ||
| LE_BOT_APP_ID: | ||
| description: 'GitHub App ID for authentication' | ||
| required: true | ||
| LE_BOT_PRIVATE_KEY: | ||
| description: 'GitHub App Private Key for authentication' | ||
| required: true | ||
| jobs: | ||
| check-if-contributor: | ||
| name: Check if author is contributor | ||
| uses: learningequality/.github/.github/workflows/is-contributor.yml@main | ||
| secrets: | ||
| LE_BOT_APP_ID: ${{ secrets.LE_BOT_APP_ID }} | ||
| LE_BOT_PRIVATE_KEY: ${{ secrets.LE_BOT_PRIVATE_KEY }} | ||
| with: | ||
| username: ${{ github.event.pull_request.user.login }} | ||
| author_association: ${{ github.event.pull_request.author_association }} | ||
| handle-review-requested: | ||
| name: Handle review requested | ||
| needs: [check-if-contributor] | ||
| if: ${{ needs.check-if-contributor.outputs.is_contributor == 'true' }} | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Generate App Token | ||
| id: generate-token | ||
| uses: actions/create-github-app-token@v2 | ||
| with: | ||
| app-id: ${{ secrets.LE_BOT_APP_ID }} | ||
| private-key: ${{ secrets.LE_BOT_PRIVATE_KEY }} | ||
| - name: Checkout .github repository | ||
| uses: actions/checkout@v4 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ibid |
||
| with: | ||
| repository: learningequality/.github | ||
| ref: main | ||
| token: ${{ steps.generate-token.outputs.token }} | ||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ibid |
||
| with: | ||
| node-version: '20' | ||
| - name: Install dependencies | ||
| run: yarn install --frozen-lockfile | ||
| - name: Run script | ||
| id: script | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| github-token: ${{ steps.generate-token.outputs.token }} | ||
| script: | | ||
| const script = require('./scripts/review-requested.js'); | ||
| return await script({ github, context, core }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # Community Review | ||
|
|
||
| ## Authors | ||
|
|
||
| 🎉 _Thank you for your contribution! Before we assign a maintainer, we invite the open-source community to pre-review your pull request. The focus of this round is community collaboration. Be kind with each other & have fun!_ :relieved: | ||
|
|
||
| - Always engage with the community **respectfully and patiently**, and in line with our [Contributing guidelines](https://learningequality.org/contributing-to-our-open-code-base). Pay extra attention to [Using generative AI](https://learningequality.org/contributing-to-our-open-code-base/#using-generative-ai)—this is an opportunity to practice your own communication and collaboration skills. | ||
| - Experience levels in the community vary. **You don’t need to treat all feedback as final—discuss and clarify together. If something is unclear, just wait for a maintainer review.** | ||
| - If you notice any inappropriate behavior, contact us at [contributors@learningequality.org](mailto:contributors@learningequality.org). | ||
|
|
||
| ## Reviewers | ||
|
|
||
| 🙌 _Thank you for joining the review! Based on your interest and experience, you may choose to do some or all of the following: compare the implementation with the issue requirements, run the pull request locally and confirm it works as expected, report bugs, or ask questions. If you didn’t spot any problems, give the authors a pat on the back! Be kind with each other & have fun!_ :relieved: | ||
|
|
||
| - Always keep in mind: **Respectful, Concise, Relevant** | ||
| - Follow our [Contributing guidelines](https://learningequality.org/contributing-to-our-open-code-base). **Pay extra attention to [Using generative AI](https://learningequality.org/contributing-to-our-open-code-base/#using-generative-ai). Avoid generic & long or otherwise AI-generated comments—they aren’t helpful and feel disrespectful to both authors and maintainers.** This is an opportunity to practice your own communication and collaboration skills. | ||
| - You may want to briefly introduce yourself and mention your experience level | ||
| - Before posting your first review, **see the following resources**: | ||
| - [What makes a code review good or bad?](https://github.blog/developer-skills/github/how-to-review-code-effectively-a-github-staff-engineers-philosophy/#what-makes-a-code-review-good-or-bad) | ||
| - [How to give a good code review](https://github.blog/developer-skills/github/how-to-review-code-effectively-a-github-staff-engineers-philosophy/#how-to-give-a-good-code-review) | ||
| * If you notice any inappropriate behavior, contact us at [contributors@learningequality.org](mailto:contributors@learningequality.org). | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: This line uses |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // Send info message when community-review label is added | ||
|
|
||
| const { BOT_MESSAGE_COMMUNITY_REVIEW, LABEL_COMMUNITY_REVIEW } = require('./constants'); | ||
| const { sendBotMessage } = require('./utils'); | ||
|
|
||
| module.exports = async ({ github, context, core }) => { | ||
| try { | ||
| const label = context.payload.label?.name; | ||
| if (label !== LABEL_COMMUNITY_REVIEW) { | ||
| return; | ||
| } | ||
| const prNumber = context.payload.pull_request.number; | ||
| await sendBotMessage(prNumber, BOT_MESSAGE_COMMUNITY_REVIEW, { github, context }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Unlike Not blocking since it's an edge case that requires manual label toggling, but worth considering.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's valid concern, but I don't think we need to introduce more complexity to take care of an action that we'll rarely, if ever, do.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough — it's a rare edge case and not worth the added complexity. Withdrawing this suggestion. |
||
| } catch (error) { | ||
| core.setFailed(`Action failed with error: ${error.message}`); | ||
| } | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // Send info message about rtibblesbot review | ||
|
|
||
| const { BOT_MESSAGE_RTIBBLESBOT_REVIEW, RTIBBLESBOT_USERNAME } = require('./constants'); | ||
| const { sendBotMessage } = require('./utils'); | ||
|
|
||
| module.exports = async ({ github, context, core }) => { | ||
| try { | ||
| const reviewer = context.payload.requested_reviewer?.login; | ||
| if (reviewer !== RTIBBLESBOT_USERNAME) { | ||
| return; | ||
| } | ||
| const prNumber = context.payload.pull_request.number; | ||
| await sendBotMessage(prNumber, BOT_MESSAGE_RTIBBLESBOT_REVIEW, { github, context }); | ||
| } catch (error) { | ||
| core.setFailed(`Action failed with error: ${error.message}`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rtibbles I tried to use the event-based handler structure for the two new workflows, rather than having separate
call-.....ymlfor each workflow as you suggested, and it seems to work nice ~ thank you. It will definitely save us some time & headaches with maintaining the five repos that call it.Even though if any new workflows based on pull request target event are added in the future, we may still need to update
types:five times. Because not specifyingtypesat all defaults to few default types that weren't sufficient for this use-case. Or we could specify all types right away, but that feelt rather unnecessary.. In any case, even with this small hiccup, it'll still be much easier!Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have space to refactor all existing workflows right now to use this handler, but it's tracked in Notion, so I will eventually work my way towards it.