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

Move wip checks to github actions #276

Merged
merged 7 commits into from
Aug 1, 2021

Conversation

jameesjohn
Copy link
Contributor

Explanation

Moves WIP/Draft checks to github actions

These features have been tested at jameesjohn/comment-on-pr#51 (comment)

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.

Comment on lines 53 to 54
case constants.wipCheck:
await wipDraftModule.checkWIP();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
case constants.wipCheck:
await wipDraftModule.checkWIP();
case constants.wipCheck:
core.info('wip check triggerred');
await wipDraftModule.checkWIP();

can we have something like this? This is to make debugging easier and also we'll know what was executed in the action.

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.

@oppiabot
Copy link

oppiabot bot commented Jun 29, 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!

Copy link
Contributor

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

actions/src/utils.js Outdated Show resolved Hide resolved
* @param {string[]} assignees
* @param {string} comment
*/
const pingAndAssignUsers = async (octokit, pullRequest, assignees, comment) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function doesn't really ping users, it just creates comment (which might contain entirely different content).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it also assigns the users. Comments can be created directly. This function helps when we want to make the comment and also assign users at the same time.

actions/src/pull_requests/checkWipDraftPR.js Outdated Show resolved Hide resolved
actions/src/pull_requests/checkWipDraftPR.js Outdated Show resolved Hide resolved
jameesjohn and others added 2 commits July 19, 2021 11:49
Co-authored-by: Vojtěch Jelínek <vojtin.j@gmail.com>
Co-authored-by: Vojtěch Jelínek <vojtin.j@gmail.com>
@jameesjohn
Copy link
Contributor Author

PTAL @vojtechjelinek

@oppiabot
Copy link

oppiabot bot commented Jul 27, 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!

Copy link
Contributor

@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!

@vojtechjelinek vojtechjelinek merged commit d343d80 into oppia:master Aug 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants