Fix #217: Removes PR from Blocking Bugs Milestone #249

SAEb-ai · 2021-04-06T10:54:58Z

Explanation

Fixes #217. It removes PR from Blocking Bugs Milestone

Checklist

I have successfully deployed my own instance of Oppiabot.
- You can find instructions for doing this here.
I have manually tested all the changes made in this PR following the manual tests matrix.

SAEb-ai · 2021-04-08T08:00:19Z

@jameesjohn @vojtechjelinek PTAL. I just want to get the confirmation that the method which I have adopted to remove milestone from PR looks good or not and then I will try to work on the failure of frontend tests.
Thanks!

vojtechjelinek · 2021-04-08T11:59:15Z

The approach looks correct.

jameesjohn

Thanks, @SAEb-ai!
I took the first pass.

jameesjohn · 2021-04-16T07:56:49Z

lib/checkPullRequestLabels.js

@@ -31,8 +31,10 @@ const {
  oppiaDevWorkflowTeam,
  serverJobAdmins,
 } = require('../userWhitelist.json');
+const { Octokit } = require('probot');


Is this required?

jameesjohn · 2021-04-16T08:25:53Z

index.js

+          case constants.prMilestoneCheck:
+            callable.push(
+              checkPullRequestLabelsModule.checkForMilestone(context)
+            )


Suggested change

)

);

jameesjohn · 2021-04-16T08:26:56Z

constants.js

@@ -143,6 +144,7 @@ module.exports.issuesLabelCheck = issuesLabelCheck;
 module.exports.issuesAssignedCheck = issuesAssignedCheck;
 module.exports.datastoreLabelCheck = datastoreLabelCheck;
 module.exports.prLabelCheck = prLabelCheck;
+module.exports.prMilestoneCheck = prMilestoneCheck;


You should probably add the milestoned event and specify that the prMilestoneCheck be run.
See how other events like the openEvent were done.

oppiabot/constants.js

Lines 71 to 74 in ec404f4

'oppia': {

[openEvent]: [

claCheck,

changelogCheck,

jameesjohn · 2021-04-16T08:28:27Z

index.js

@@ -110,6 +110,11 @@ const runChecks = async (context, checkEvent) => {
              checkPullRequestLabelsModule.checkForIssueLabel(context)


Since this feature listens for a new event, there should be an entry where oppiabot listens for the event. Similar to

oppiabot/index.js

Lines 237 to 241 in ec404f4

oppiabot.on('pull_request.labeled', async (context) => {

if (checkWhitelistedAccounts(context) && checkAuthor(context)) {

await runChecks(context, constants.PRLabelEvent);

}

});

jameesjohn · 2021-04-16T08:29:44Z

lib/checkPullRequestLabels.js

+    const commentBody =
+      'Hi @' + user.login + ', the ' + milestone.title +
+      ' milestone should only be ' +
+      'used on issues, and Im removing the milestone. Thanks!';


Suggested change

'used on issues, and Im removing the milestone. Thanks!';

'used on issues, and I'm removing the milestone. Thanks!';

oppiabot · 2021-04-16T18:14:47Z

Hi @SAEb-ai. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

SAEb-ai · 2021-04-16T19:15:51Z

@jameesjohn @vojtechjelinek PTAL

vojtechjelinek

Thanks! Left two comments.

vojtechjelinek · 2021-04-19T13:36:23Z

lib/checkPullRequestLabels.js

@@ -33,6 +33,7 @@ const {
 } = require('../userWhitelist.json');
 const DEFAULT_CHANGELOG_LABEL = 'REVIEWERS: Please add changelog label';
 const PR_LABELS = ['dependencies', 'stale', DEFAULT_CHANGELOG_LABEL];
+const MILESTONES = ['Blocking Bugs'];


Can we get the milestones from GitHub? If not this should be probably added as a constant in constants.js.

Added it in constants.js. Done!

vojtechjelinek · 2021-04-19T13:36:59Z

lib/checkPullRequestLabels.js

+    const commentBody =
+      'Hi @' + user.login + ', the ' + milestone.title +
+      ' milestone should only be ' +
+      'used on issues, and I’m removing the milestone. Thanks!';


Suggested change

'used on issues, and I’m removing the milestone. Thanks!';

'used for issues. I’m removing the milestone from this PR. Thanks!';

SAEb-ai · 2021-08-26T13:47:15Z

@vojtechjelinek @jameesjohn @gp201 PTAL.Thanks!

oppiabot · 2021-08-26T13:50:18Z

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

SAEb-ai · 2021-08-26T13:50:37Z

constants.js

+const milestones = ['Regressions (August 2021)',
+  'Hotlist (August 2021)',
+  'Py3 Migration QA (June 2021)'];


@jameesjohn Can we extract the milestones of a repository from github API because this doesn't look to be an optimal solution?

PTAL https://docs.github.com/en/rest/reference/issues#milestones

gp201 · 2021-08-26T13:53:31Z

lib/checkPullRequestLabels.js

+ * This function checks that a milestone has not been added to a
+ * Pull Request.


Suggested change

* This function checks that a milestone has not been added to a

* Pull Request.

* This function ensures that a milestone has not been added to a

* Pull Request.

gp201 · 2021-08-26T13:55:16Z

spec/checkPullRequestLabelsSpec.js

@@ -464,6 +465,42 @@ describe('Pull Request Label Check', () => {
    });
  });

+  describe('when a pr is milestoned', () => {


Can you also write a test to make sure nothing happens when a milestone is not added?

gp201 · 2021-08-26T14:05:25Z

constants.js

+const milestones = ['Regressions (August 2021)',
+  'Hotlist (August 2021)',
+  'Py3 Migration QA (June 2021)'];


PTAL https://docs.github.com/en/rest/reference/issues#milestones

oppiabot · 2021-08-26T14:13:00Z

Unassigning @gp201 since the review is done.

oppiabot · 2021-08-26T14:13:03Z

Hi @SAEb-ai, it looks like some changes were requested on this pull request by @gp201. PTAL. Thanks!

This reverts commit fe40e2b.

oppiabot · 2021-10-04T14:50:12Z

Hi @SAEb-ai. Due to recent changes in the "develop" branch, this PR now has a merge conflict. Please follow this link if you need help resolving the conflict, so that the PR can be merged. Thanks!

This reverts commit cecae47.

This reverts commit c67d2b4.

SAEb-ai added 6 commits April 6, 2021 15:59

Fixed Lint Issues

1488001

Remove Pr from Blocking Bugs

1661e32

Fixed minors

b360614

Debugging

48b23b9

Added checkForMilestone in respective files

30ef878

Resolved nits

8f9f2b7

SAEb-ai marked this pull request as ready for review April 8, 2021 07:57

vojtechjelinek assigned vojtechjelinek and jameesjohn Apr 8, 2021

vojtechjelinek requested review from vojtechjelinek and jameesjohn and removed request for vojtechjelinek April 8, 2021 11:58

Testing

50868df

jameesjohn suggested changes Apr 16, 2021

View reviewed changes

Addressed Review Comments

ed2158b

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Apr 16, 2021

oppiabot bot assigned SAEb-ai Apr 16, 2021

SAEb-ai requested a review from jameesjohn April 16, 2021 19:15

SAEb-ai removed their assignment Apr 16, 2021

vojtechjelinek requested changes Apr 19, 2021

View reviewed changes

vojtechjelinek assigned SAEb-ai and unassigned vojtechjelinek and jameesjohn Apr 20, 2021

SAEb-ai added 2 commits August 26, 2021 17:17

fix conflicts

02ac0ff

address

8e46392

oppiabot bot removed the PR: don't merge - HAS MERGE CONFLICTS label Aug 26, 2021

address

96a6c8f

SAEb-ai requested a review from vojtechjelinek August 26, 2021 13:46

frontend

485be0c

oppiabot bot assigned gp201, jameesjohn and vojtechjelinek and unassigned SAEb-ai Aug 26, 2021

SAEb-ai commented Aug 26, 2021

View reviewed changes

gp201 reviewed Aug 26, 2021

View reviewed changes

gp201 suggested changes Aug 26, 2021

View reviewed changes

oppiabot bot unassigned gp201 Aug 26, 2021

oppiabot bot assigned SAEb-ai Aug 26, 2021

SAEb-ai added 2 commits October 1, 2021 23:23

Test

fe40e2b

Revert

c67d2b4

This reverts commit fe40e2b.

oppiabot bot added the PR: don't merge - HAS MERGE CONFLICTS label Oct 4, 2021

SAEb-ai added 4 commits October 5, 2021 11:43

Test

cecae47

Revert "Test"

4737208

This reverts commit cecae47.

Revert "Revert"

388d1ad

This reverts commit c67d2b4.

test

86225c6

vojtechjelinek removed their assignment Oct 5, 2021

vojtechjelinek unassigned jameesjohn Feb 2, 2022

vojtechjelinek closed this Aug 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #217: Removes PR from Blocking Bugs Milestone #249

Fix #217: Removes PR from Blocking Bugs Milestone #249

SAEb-ai commented Apr 6, 2021 •

edited

Loading

SAEb-ai commented Apr 8, 2021

vojtechjelinek commented Apr 8, 2021

jameesjohn left a comment

jameesjohn Apr 16, 2021

SAEb-ai Apr 16, 2021

jameesjohn Apr 16, 2021

SAEb-ai Apr 16, 2021

jameesjohn Apr 16, 2021

SAEb-ai Apr 16, 2021

jameesjohn Apr 16, 2021

SAEb-ai Apr 16, 2021

jameesjohn Apr 16, 2021

SAEb-ai Apr 16, 2021

oppiabot bot commented Apr 16, 2021

SAEb-ai commented Apr 16, 2021

vojtechjelinek left a comment

vojtechjelinek Apr 19, 2021

SAEb-ai Aug 26, 2021

vojtechjelinek Apr 19, 2021

SAEb-ai Aug 26, 2021

SAEb-ai commented Aug 26, 2021

oppiabot bot commented Aug 26, 2021

SAEb-ai Aug 26, 2021

gp201 Aug 26, 2021

gp201 Aug 26, 2021

gp201 Aug 26, 2021

gp201 Aug 26, 2021

oppiabot bot commented Aug 26, 2021

oppiabot bot commented Aug 26, 2021

oppiabot bot commented Oct 4, 2021

		@@ -110,6 +110,11 @@ const runChecks = async (context, checkEvent) => {
		checkPullRequestLabelsModule.checkForIssueLabel(context)

	oppiabot.on('pull_request.labeled', async (context) => {
	if (checkWhitelistedAccounts(context) && checkAuthor(context)) {
	await runChecks(context, constants.PRLabelEvent);
	}
	});

	'used on issues, and Im removing the milestone. Thanks!';
	'used on issues, and I'm removing the milestone. Thanks!';

	'used on issues, and I’m removing the milestone. Thanks!';
	'used for issues. I’m removing the milestone from this PR. Thanks!';

		* This function checks that a milestone has not been added to a
		* Pull Request.

Fix #217: Removes PR from Blocking Bugs Milestone #249

Fix #217: Removes PR from Blocking Bugs Milestone #249

Conversation

SAEb-ai commented Apr 6, 2021 • edited Loading

Explanation

Checklist

SAEb-ai commented Apr 8, 2021

vojtechjelinek commented Apr 8, 2021

jameesjohn left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oppiabot bot commented Apr 16, 2021

SAEb-ai commented Apr 16, 2021

vojtechjelinek left a 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

SAEb-ai commented Aug 26, 2021

oppiabot bot commented Aug 26, 2021

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

oppiabot bot commented Aug 26, 2021

oppiabot bot commented Aug 26, 2021

oppiabot bot commented Oct 4, 2021

SAEb-ai commented Apr 6, 2021 •

edited

Loading