Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/call-pull-request-target.yml
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]
Copy link
Member Author

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-.....yml for 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 specifying types at 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!

Copy link
Member Author

@MisRob MisRob Feb 23, 2026

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.

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 }}
52 changes: 52 additions & 0 deletions .github/workflows/pull-request-label.yml
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
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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 });
25 changes: 25 additions & 0 deletions .github/workflows/pull-request-target.yml
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:
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 }}
52 changes: 52 additions & 0 deletions .github/workflows/review-requested.yml
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
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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 });
21 changes: 21 additions & 0 deletions docs/community-review.md
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).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: This line uses * while all other bullet points in the file use -.

19 changes: 17 additions & 2 deletions scripts/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@
const LE_BOT_USERNAME = 'learning-equality-bot[bot]';
const SENTRY_BOT_USERNAME = 'sentry-io[bot]';
const DEPENDABOT_USERNAME = 'dependabot[bot]';
const RTIBBLESBOT_USERNAME = 'rtibblesbot';
const BOT_USERNAMES = [
LE_BOT_USERNAME,
SENTRY_BOT_USERNAME,
DEPENDABOT_USERNAME,
RTIBBLESBOT_USERNAME,
];

// close contributors are treated a bit special in some workflows,
// for example, we receive a high priority notification about their
Expand Down Expand Up @@ -75,6 +82,7 @@ const KEYWORDS_DETECT_ASSIGNMENT_REQUEST = [
];

const ISSUE_LABEL_HELP_WANTED = 'help wanted';
const LABEL_COMMUNITY_REVIEW = 'community-review';

// Will be attached to bot messages when not empty
// const GSOC_NOTE = '';
Expand All @@ -88,6 +96,10 @@ const BOT_MESSAGE_PULL_REQUEST = `👋 Thanks for contributing! \n\n We will ass

const HOLIDAY_MESSAGE = `Season's greetings! 👋 \n\n We'd like to thank everyone for another year of fruitful collaborations, engaging discussions, and for the continued support of our work. **Learning Equality will be on holidays from December 22 to January 5.** We look forward to much more in the new year and wish you a very happy holiday season!${GSOC_NOTE}`;

const BOT_MESSAGE_RTIBBLESBOT_REVIEW = `📢✨ **Before we assign a reviewer, we'll turn on \`@rtibblesbot\` to pre-review. Its comments are generated by an LLM, and should be evaluated accordingly.**`;

const BOT_MESSAGE_COMMUNITY_REVIEW = `📢✨ **Before we assign a reviewer, we'll invite community pre-review. See the [community review guidance](https://github.com/learningequality/.github/blob/main/docs/community-review.md) for both authors and reviewers.**`;

// Repositories to include in PR statistics reports
const PR_STATS_REPOS = [
'kolibri',
Expand All @@ -103,14 +115,17 @@ const COMMUNITY_REPOS = [...PR_STATS_REPOS];

module.exports = {
LE_BOT_USERNAME,
SENTRY_BOT_USERNAME,
DEPENDABOT_USERNAME,
BOT_USERNAMES,
CLOSE_CONTRIBUTORS,
KEYWORDS_DETECT_ASSIGNMENT_REQUEST,
ISSUE_LABEL_HELP_WANTED,
BOT_MESSAGE_ISSUE_NOT_OPEN,
BOT_MESSAGE_ALREADY_ASSIGNED,
BOT_MESSAGE_PULL_REQUEST,
BOT_MESSAGE_RTIBBLESBOT_REVIEW,
RTIBBLESBOT_USERNAME,
LABEL_COMMUNITY_REVIEW,
BOT_MESSAGE_COMMUNITY_REVIEW,
TEAMS_WITH_CLOSE_CONTRIBUTORS,
HOLIDAY_MESSAGE,
PR_STATS_REPOS,
Expand Down
17 changes: 17 additions & 0 deletions scripts/pull-request-label.js
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 });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Unlike review_requested, the labeled event can fire multiple times for the same label (remove label → re-add). This would post duplicate bot comments. Consider adding a hasRecentBotComment check like holiday-message.js does, or checking if a comment with this message already exists on the PR.

Not blocking since it's an edge case that requires manual label toggling, but worth considering.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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}`);
}
};
17 changes: 17 additions & 0 deletions scripts/review-requested.js
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}`);
}
};
4 changes: 2 additions & 2 deletions scripts/utils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { LE_BOT_USERNAME, SENTRY_BOT_USERNAME, DEPENDABOT_USERNAME } = require('./constants');
const { BOT_USERNAMES } = require('./constants');
// const { CLOSE_CONTRIBUTORS, TEAMS_WITH_CLOSE_CONTRIBUTORS } = require('./constants');
const { CLOSE_CONTRIBUTORS } = require('./constants');

Expand All @@ -10,7 +10,7 @@ async function isBot(username, { core }) {
core.setFailed('Missing username');
return false;
}
return [LE_BOT_USERNAME, SENTRY_BOT_USERNAME, DEPENDABOT_USERNAME].includes(username);
return BOT_USERNAMES.includes(username);
}

/**
Expand Down