Fix #20381 : Added Github Action for developer Onboarding leads #20683

HardikGoyal2003 · 2024-07-16T15:25:05Z

Overview

This PR fixes or fixes part of [Feature Request]: Notify developer onboarding leads about new contributors merged PRs #20381
This PR does the following: Adds the GitHub action which will trigger when a PR gets merged in develop branch

Essential Checklist

Please follow the instructions for making a code change.

I have linked the issue that this PR fixes in the "Development" section of the sidebar.
I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
I have written tests for my code.
The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Proof that changes are correct

PR Pointers

Never force push! If you do, your PR will be closed.
To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
Some e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.
See the Code Owner's wiki page for what code owners will expect.

oppiabot · 2024-07-16T15:25:17Z

Assigning @U8NWXD for the first pass review of this PR. Thanks!

HardikGoyal2003 · 2024-07-16T15:27:00Z

@seanlip This is the comment that needs to watch out from previous opened Pr #20382 (comment)

HardikGoyal2003 · 2024-07-18T13:00:27Z

@Ash-2k3 @seanlip @U8NWXD PTAL! Thanks!

.github/workflows/developer_onboarding_notification.yml

seanlip · 2024-07-19T15:54:16Z

.github/workflows/developer_onboarding_notification.yml

+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - uses: ./.github/actions/merge


@HardikGoyal2003 Re #20382 (comment), fair enough, but then should we give this step a name to explain what it actually does, just like the other steps?

Hey @U8NWXD Please can you explain, I looked at the code but couldn't get it.

I didn't leave this comment, but if you're asking how to give the step a name, take a look at the immediately preceding step:

- name: Checkout repository uses: actions/checkout@v4

Checkout repository is the name. If you're confused by the YAML syntax, here's the spec: https://yaml.org/spec/1.2.2/

@U8NWXD No, I think I was unclear, what I wanted to ask is what - uses: ./.github/actions/merge this does.

Here's the code for reference:

oppia/.github/actions/merge/action.yml

Lines 1 to 26 in 0e6d986

name: 'Merge Source Branch into Base Branch'

description: 'Merge the PR source branch into its base branch, leaving the merge commit checked-out'

runs:

using: 'composite'

steps:

- id: merge

name: Merge

run: |

if [[ ${{ github.event_name}} != "pull_request" ]]

then

echo "Skipping merge because not triggered by a PR event."

exit 0

fi

echo "Base Repository: ${{ github.repository }}"

echo "Base Branch: ${{ github.base_ref }}"

echo "Head Repository: ${{ github.event.pull_request.head.repo.full_name }}"

echo "Head Branch: $GITHUB_HEAD_REF"

git config --global user.email "mergeAction@example.com"

git config --global user.name "Oppia Merge Base Branch GitHub Action"

git remote add source "https://github.com/${{ github.event.pull_request.head.repo.full_name }}.git"

git remote add base "https://github.com/${{ github.repository }}.git"

git fetch source $GITHUB_HEAD_REF

git fetch base ${{ github.base_ref }}

git checkout base/${{ github.base_ref }}

git merge source/$GITHUB_HEAD_REF

shell: bash

The branch that the PR was opened against (usually develop) is called the base. The branch that has the changes in the PR (in this case, onboarding) is called the source. This action checks out the base branch and merges in the source. This mimics what would happen when the PR is merged. It then leaves the merge commit checked-out so that all the tests run against this simulated post-merge code.

We added this step so that if we fixed flaky tests in develop, PRs would automatically incorporate those fixes as soon as the tests were rerun. Otherwise, PR authors would have to manually merge with develop and rerun all the tests.

@HardikGoyal2003 -- ah, thanks. Do you know where the linter error originates and which PR introduced that check? I ask because this might be the first workflow we've written that doesn't actually need it, so we might want to consider adding it as an exception to that lint check rather than putting code in here that doesn't make sense.

@seanlip This PR introduced it. and this is the function. Thanks!

OK, thanks @HardikGoyal2003. I think, let's leave a comment in this step to explain it's not actually needed for this workflow, but is mandated by the lint check _check_that_workflow_steps_use_merge_action() (and point to PR 13700). I'll talk to @U8NWXD about whether we should do anything long-term here. Thanks!

@HardikGoyal2003 I talked to @U8NWXD about this, and there is actually a constant here that you can put this workflow in to exempt it from the lint check in question.

So I want to amend my suggestion in the previous comment -- please instead add your workflow to that list, and drop the step altogether if this workflow does not need it. Thanks!

HardikGoyal2003 · 2024-07-23T17:36:48Z

@seanlip @U8NWXD PTAL!! Thanks!

oppiabot · 2024-07-23T17:39:52Z

Unassigning @HardikGoyal2003 since a re-review was requested. @HardikGoyal2003, please make sure you have addressed all review comments. Thanks!

U8NWXD · 2024-07-24T01:54:45Z

Responded!

HardikGoyal2003 · 2024-07-24T15:00:09Z

@U8NWXD PTAL!! Thanks!

oppiabot · 2024-07-29T16:42:46Z

Unassigning @HardikGoyal2003 since a re-review was requested. @HardikGoyal2003, please make sure you have addressed all review comments. Thanks!

HardikGoyal2003 · 2024-08-01T18:00:04Z

Hey @Ash-2k3 PTAL!! Thanks!

HardikGoyal2003 · 2024-08-02T11:03:19Z

@seanlip PTAL!! Thanks!

oppiabot · 2024-08-02T11:06:23Z

Unassigning @HardikGoyal2003 since a re-review was requested. @HardikGoyal2003, please make sure you have addressed all review comments. Thanks!

HardikGoyal2003 · 2024-08-07T02:33:45Z

@seanlip PTAL!! Thanks!

oppiabot · 2024-08-07T02:36:48Z

Unassigning @HardikGoyal2003 since a re-review was requested. @HardikGoyal2003, please make sure you have addressed all review comments. Thanks!

oppiabot · 2024-08-07T13:39:03Z

Unassigning @seanlip since they have already approved the PR.

Ash-2k3

@HardikGoyal2003 Sorry for the late review, I was quite busy with my internship and Oppia's Contract! Thanks for this creating this action, I think this is a great add on and will help us to keep track of new contributors so that we could help them. I just have one comment, more like a suggestion, PTAL.

Ash-2k3 · 2024-08-07T19:27:30Z

.github/workflows/developer_onboarding_notification.yml

+          script: |
+            const prCount = parseInt(process.env.PR_COUNT);
+            const author = process.env.AUTHOR;
+            const mention = 'HardikGoyal2003';  // Update this if the mention needs to change


Can we include more mentions to ensure it gets seen, if in case one of the mentions is not active ? WDYT ?

We can add as many as you like. I initially included Afzal, but removed him due to unavailability. Once we have more onboarding leads or additional members in the welfare team, please let me know, and I will update it. Thanks!

Added developer notification

fce0a6b

HardikGoyal2003 requested a review from a team as a code owner July 16, 2024 15:25

HardikGoyal2003 requested a review from U8NWXD July 16, 2024 15:25

oppiabot bot assigned U8NWXD Jul 16, 2024

HardikGoyal2003 assigned seanlip and unassigned U8NWXD Jul 16, 2024

HardikGoyal2003 requested review from seanlip and Ash-2k3 July 16, 2024 15:25

HardikGoyal2003 assigned Ash-2k3 and U8NWXD Jul 16, 2024

U8NWXD requested changes Jul 19, 2024

View reviewed changes

.github/workflows/developer_onboarding_notification.yml Show resolved Hide resolved

.github/workflows/developer_onboarding_notification.yml Outdated Show resolved Hide resolved

.github/workflows/developer_onboarding_notification.yml Show resolved Hide resolved

U8NWXD assigned HardikGoyal2003 and unassigned U8NWXD Jul 19, 2024

seanlip requested changes Jul 19, 2024

View reviewed changes

seanlip removed their assignment Jul 19, 2024

Made minor changes

49404b9

oppiabot bot assigned seanlip and U8NWXD and unassigned HardikGoyal2003 Jul 23, 2024

U8NWXD assigned HardikGoyal2003 and unassigned U8NWXD Jul 24, 2024

HardikGoyal2003 assigned U8NWXD and unassigned HardikGoyal2003 Jul 24, 2024

oppiabot bot unassigned HardikGoyal2003 Jul 29, 2024

U8NWXD approved these changes Jul 30, 2024

View reviewed changes

U8NWXD removed their assignment Jul 30, 2024

seanlip assigned HardikGoyal2003 and unassigned seanlip Aug 1, 2024

oppiabot bot assigned seanlip Aug 2, 2024

oppiabot bot unassigned HardikGoyal2003 Aug 2, 2024

seanlip assigned HardikGoyal2003 and unassigned seanlip Aug 6, 2024

Added exception in linters

b448786

oppiabot bot assigned seanlip and unassigned HardikGoyal2003 Aug 7, 2024

seanlip approved these changes Aug 7, 2024

View reviewed changes

seanlip added this pull request to the merge queue Aug 7, 2024

oppiabot bot unassigned seanlip Aug 7, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 7, 2024

HardikGoyal2003 added this pull request to the merge queue Aug 7, 2024

Merged via the queue into oppia:develop with commit 2f3683c Aug 7, 2024
125 checks passed

HardikGoyal2003 deleted the onboarding branch August 7, 2024 17:06

Ash-2k3 reviewed Aug 7, 2024

View reviewed changes

HardikGoyal2003 mentioned this pull request Aug 19, 2024

[Feature Request]: Notify developer onboarding leads about new contributors merged PRs #20381

Closed

HardikGoyal2003 mentioned this pull request Oct 3, 2024

[Feature Request]: Implement messages to celebrate successful first- and second-PR contributions oppia/oppia-android#5547

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #20381 : Added Github Action for developer Onboarding leads #20683

Fix #20381 : Added Github Action for developer Onboarding leads #20683

HardikGoyal2003 commented Jul 16, 2024

oppiabot bot commented Jul 16, 2024

HardikGoyal2003 commented Jul 16, 2024

HardikGoyal2003 commented Jul 18, 2024

seanlip Jul 19, 2024

HardikGoyal2003 Jul 23, 2024

U8NWXD Jul 24, 2024

HardikGoyal2003 Jul 24, 2024

U8NWXD Jul 26, 2024

seanlip Aug 1, 2024

HardikGoyal2003 Aug 2, 2024

seanlip Aug 6, 2024

seanlip Aug 7, 2024

HardikGoyal2003 Aug 7, 2024

HardikGoyal2003 commented Jul 23, 2024

oppiabot bot commented Jul 23, 2024

U8NWXD commented Jul 24, 2024

HardikGoyal2003 commented Jul 24, 2024

oppiabot bot commented Jul 29, 2024

HardikGoyal2003 commented Aug 1, 2024

HardikGoyal2003 commented Aug 2, 2024

oppiabot bot commented Aug 2, 2024

HardikGoyal2003 commented Aug 7, 2024

oppiabot bot commented Aug 7, 2024

oppiabot bot commented Aug 7, 2024

Ash-2k3 left a comment

Ash-2k3 Aug 7, 2024

HardikGoyal2003 Aug 8, 2024

	name: 'Merge Source Branch into Base Branch'
	description: 'Merge the PR source branch into its base branch, leaving the merge commit checked-out'
	runs:
	using: 'composite'
	steps:
	- id: merge
	name: Merge
	run: \|
	if [[ ${{ github.event_name}} != "pull_request" ]]
	then
	echo "Skipping merge because not triggered by a PR event."
	exit 0
	fi
	echo "Base Repository: ${{ github.repository }}"
	echo "Base Branch: ${{ github.base_ref }}"
	echo "Head Repository: ${{ github.event.pull_request.head.repo.full_name }}"
	echo "Head Branch: $GITHUB_HEAD_REF"
	git config --global user.email "mergeAction@example.com"
	git config --global user.name "Oppia Merge Base Branch GitHub Action"
	git remote add source "https://github.com/${{ github.event.pull_request.head.repo.full_name }}.git"
	git remote add base "https://github.com/${{ github.repository }}.git"
	git fetch source $GITHUB_HEAD_REF
	git fetch base ${{ github.base_ref }}
	git checkout base/${{ github.base_ref }}
	git merge source/$GITHUB_HEAD_REF
	shell: bash

Fix #20381 : Added Github Action for developer Onboarding leads #20683

Fix #20381 : Added Github Action for developer Onboarding leads #20683

Conversation

HardikGoyal2003 commented Jul 16, 2024

Overview

Essential Checklist

Proof that changes are correct

PR Pointers

oppiabot bot commented Jul 16, 2024

HardikGoyal2003 commented Jul 16, 2024

HardikGoyal2003 commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HardikGoyal2003 commented Jul 23, 2024

oppiabot bot commented Jul 23, 2024

U8NWXD commented Jul 24, 2024

HardikGoyal2003 commented Jul 24, 2024

oppiabot bot commented Jul 29, 2024

HardikGoyal2003 commented Aug 1, 2024

HardikGoyal2003 commented Aug 2, 2024

oppiabot bot commented Aug 2, 2024

HardikGoyal2003 commented Aug 7, 2024

oppiabot bot commented Aug 7, 2024

oppiabot bot commented Aug 7, 2024

Ash-2k3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment