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

Migrated ciChecks Module from probot to github actions #300

Closed
wants to merge 7 commits into from

Conversation

Ketan1502
Copy link

Explanation

The ciChecks.js module is successfully migrated from probot to github actions as mentioned in issue #277

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.

@Ketan1502 Ketan1502 mentioned this pull request Feb 17, 2022
16 tasks
@vojtechjelinek vojtechjelinek self-assigned this Feb 17, 2022
@Ketan1502
Copy link
Author

@vojtechjelinek please let me know if any corrections are to be made!

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! Took a pass. Why is the .env.save and package-lock.json file added?

actions/src/pull_requests/ciChecks.js Outdated Show resolved Hide resolved
actions/src/pull_requests/ciChecks.js Outdated Show resolved Hide resolved
constants.js Outdated Show resolved Hide resolved
@oppiabot
Copy link

oppiabot bot commented Feb 21, 2022

Unassigning @vojtechjelinek since the review is done.

@oppiabot
Copy link

oppiabot bot commented Feb 21, 2022

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

@Ketan1502 Ketan1502 removed their assignment Feb 21, 2022
@Ketan1502
Copy link
Author

@vojtechjelinek any further changes required?

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 one more comment.

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

oppiabot bot commented Feb 22, 2022

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

@Ketan1502 Ketan1502 removed their assignment Feb 22, 2022
@Ketan1502
Copy link
Author

@vojtechjelinek can it be merged now?

@vojtechjelinek
Copy link
Member

@Ketan1502 Did you test all these changes according to the instructions in the PR description.

@Ketan1502
Copy link
Author

@Ketan1502 Did you test all these changes according to the instructions in the PR description.

yes, all the tests have passed @vojtechjelinek

@vojtechjelinek
Copy link
Member

@Ketan1502 But did you test your changes manually?

@Ketan1502
Copy link
Author

I tested using npm test

@Ketan1502
Copy link
Author

Capture
its showing 0 failures, when i run npm test

@vojtechjelinek
Copy link
Member

@Ketan1502 The tests are failing though
image
also you need to verify that your changes are working manually see https://github.com/oppia/oppiabot/wiki/Deploying-your-own-instance-of-the-oppiabot and https://github.com/oppia/oppiabot/wiki/Manual-Tests-Matrix

@Ketan1502
Copy link
Author

Ketan1502 commented Mar 3, 2022

Hey! @vojtechjelinek, I have corrected the lint checks but facing issues in the frontend checks. I have cross checked my front end checks with the frontend checks of the sample PR and both were showing the similar errors but the sample PR's checks have passed but mine are not, can u please help me out to understand my fault.
Sample PR's frontend checks-
https://github.com/oppia/oppiabot/actions/runs/1071735395
1
2
MY PR's frontend checks-
https://github.com/oppia/oppiabot/actions/runs/1882562313
4
Capture

@oppiabot
Copy link

oppiabot bot commented Aug 2, 2023

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

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.

2 participants