Skip to content
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

Fix #205: Fails CI when a don't merge label get added on a pull request. #264

Merged
merged 6 commits into from
May 20, 2021

Conversation

jameesjohn
Copy link
Contributor

@jameesjohn jameesjohn commented May 9, 2021

Explanation

Fixes #205: Fails CI when a don't merge label gets added on a pull request.

This PR has been tested at jameesjohn/comment-on-pr#51

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.

Copy link
Member

@vojtechjelinek vojtechjelinek left a comment

Choose a reason for hiding this comment

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

Thanks! Left a few comments.

constants.js Outdated
@@ -17,8 +17,8 @@ const openEvent = 'opened';
const openEventGithubActions = 'pull_request_target_opened';
const reopenEventGithubActions = 'pull_request_target_reopened';
const reopenEvent = 'pull_request_reopened';
const unlabelEvent = 'unlabeled';
const PRLabelEvent = 'labeled';
const unlabelEvent = 'pull_request_unlabeled';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const unlabelEvent = 'pull_request_unlabeled';
const PRUnlabelEvent = 'pull_request_unlabeled';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -40,6 +41,12 @@ module.exports = {
case constants.claCheckGithubAction:
await claCheckGithubActionModule.claCheckGithubAction();
break;
case constants.prLabelCheck:
await pRLabelsModule.checkLabels();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await pRLabelsModule.checkLabels();
await PRLabelsModule.checkLabelAdded();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

await pRLabelsModule.checkLabels();
break;
case constants.dontMergeLabelCheck:
await pRLabelsModule.checkUnLabeled();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await pRLabelsModule.checkUnLabeled();
await PRLabelsModule.checkLabelRemoved();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

actions/src/pull_requests/labelCheck.js Show resolved Hide resolved
@oppiabot
Copy link

oppiabot bot commented May 15, 2021

Unassigning @vojtechjelinek since the review is done.

@oppiabot
Copy link

oppiabot bot commented May 15, 2021

Hi @jameesjohn, it looks like some changes were requested on this pull request by @vojtechjelinek. PTAL. Thanks!

@oppiabot
Copy link

oppiabot bot commented May 18, 2021

Hi @jameesjohn. 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!

@vojtechjelinek
Copy link
Member

@jameesjohn Is this ready for review?

@jameesjohn
Copy link
Contributor Author

Yes @vojtechjelinek.

Copy link
Member

@vojtechjelinek vojtechjelinek left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a GitHub Action for the "PR: don't merge…" labels
2 participants