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.

Sign up for GitHub

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

Jump to bottom

Fixes part of #133: Update dependencies #318

Merged
merged 18 commits into from
Jun 23, 2023
Merged

Fixes part of #133: Update dependencies #318

merged 18 commits into from
Jun 23, 2023

Conversation

winnie368c
Copy link
Collaborator

@winnie368c winnie368c commented Apr 23, 2023 •
edited
Loading

Explanation

Fixes part of #133. Updated from this list of outdated packages:
Screen Shot 2023-04-25 at 4 29 51 PM

Checked the changelogs for the packages whose versions were changed in the update; they were just bug fixes.

These dependencies remain to be updated:
Screen Shot 2023-04-16 at 11 34 53 PM
When updating probot:

  • Coverage reduced
  • 441 tests failing

When updating @actions/github:

  • Coverage reduced
  • 66 tests failing

Also migrated node version of GitHub Actions to v14.

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.

Examples of manual tests:
Adding merge conflict label:
Screen Shot 2023-05-02 at 11 01 23 AM
Removing merge conflict label:
Screen Shot 2023-05-02 at 11 18 58 AM
Adding stale label:
Screen Shot 2023-05-02 at 11 08 37 AM
Removing stale label:
Screen Shot 2023-05-02 at 11 39 39 AM

@winnie368c winnie368c marked this pull request as draft April 23, 2023 22:28
@winnie368c winnie368c marked this pull request as ready for review May 7, 2023 05:26
@winnie368c
Copy link
Collaborator Author

@gp201 PTAL!

@oppiabot oppiabot bot assigned gp201 May 7, 2023
@SD-13
Copy link

SD-13 commented May 14, 2023 •
edited
Loading

@gp201 a friendly reminder if you somehow missed it.

gp201
gp201 suggested changes May 15, 2023
View reviewed changes
Copy link
Member

@gp201 gp201 left a comment

Choose a reason for hiding this comment

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

Thanks, @winnie368c ! I left some comments. PTAL

package.json
"@actions/github": "^2.2.0",
"axios": "^0.21.1",
"dotenv": "^9.0.2",
Copy link
Member

Choose a reason for hiding this comment

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

This version change seems to be a big jump. Were u able to check the change logs if any functions we use have been changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gp201 Yes, most of the breaking changes are mostly just things like adding multiline parsing support: https://github.com/motdotla/dotenv/blob/master/CHANGELOG.md

package.json
"axios": "^0.21.1",
"dotenv": "^9.0.2",
"google-auth-library": "^7.0.4",
"googleapis": "^73.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

This version change seems to be a big jump. Were u able to check the change logs if any functions we use have been changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gp201 There are some of the APIs that Oppiabot seems to use that are listed under breaking changes between the two versions at https://github.com/googleapis/google-api-nodejs-client/blob/main/CHANGELOG.md but I'm not sure if they would affect the functionality of the codebase

@gp201 gp201 assigned winnie368c and unassigned gp201 May 15, 2023
@winnie368c winnie368c removed their assignment May 20, 2023
@winnie368c
Copy link
Collaborator Author

@gp201 PTAL

@oppiabot oppiabot bot assigned gp201 May 20, 2023
@gp201
Copy link
Member

gp201 commented May 25, 2023

@seanlip Can you please approve the workflows so the tests can run?

@seanlip
Copy link
Member

seanlip commented May 25, 2023

Yup done! @gp201

@seanlip seanlip removed their assignment May 25, 2023
@seanlip
Copy link
Member

seanlip commented May 25, 2023

@gp201 @winnie368c FYI the CI frontend tests are failing

@gp201
Copy link
Member

gp201 commented May 25, 2023

@winnie368c Looks like apiForSheetsSpec.js is failing PTAL. You can run the tests using the following command: npm run test More info about the tests can be found here: https://github.com/oppia/oppiabot/wiki/Automated-Tests

@gp201 gp201 removed their assignment May 25, 2023
@winnie368c
Copy link
Collaborator Author

winnie368c commented May 25, 2023 •
edited
Loading

@winnie368c Looks like apiForSheetsSpec.js is failing PTAL. You can run the tests using the following command: npm run test More info about the tests can be found here: https://github.com/oppia/oppiabot/wiki/Automated-Tests

@gp201 npm run test is passing locally:
Screen Shot 2023-05-25 at 2 59 45 PM
I don't see where it says apiForSheetsSpec.js is failing?

@oppiabot oppiabot bot assigned gp201 and unassigned winnie368c May 25, 2023
@oppiabot
Copy link

oppiabot bot commented May 25, 2023

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

@gp201
Copy link
Member

gp201 commented May 26, 2023

@winnie368c Looks like branch coverage is not 100% for index.js PTAL
image

@gp201 gp201 assigned winnie368c and unassigned gp201 May 27, 2023
@winnie368c
Copy link
Collaborator Author

@gp201 PTAL!

@oppiabot oppiabot bot assigned gp201 and unassigned winnie368c Jun 12, 2023
@oppiabot
Copy link

oppiabot bot commented Jun 12, 2023

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

gp201
gp201 suggested changes Jun 13, 2023
View reviewed changes
Copy link
Member

@gp201 gp201 left a comment

Choose a reason for hiding this comment

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

@winnie368c since the nodejs version has been updated, can you do the manual test matrix again?

.github/workflows/frontend_tests.yml Outdated
@@ -17,7 +17,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
Copy link
Member

Choose a reason for hiding this comment

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

Can you update this action to the latest version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@oppiabot oppiabot bot unassigned gp201 Jun 13, 2023
@oppiabot
Copy link

oppiabot bot commented Jun 13, 2023

Unassigning @gp201 since the review is done.

@oppiabot
Copy link

oppiabot bot commented Jun 13, 2023

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

@winnie368c
Copy link
Collaborator Author

@winnie368c since the nodejs version has been updated, can you do the manual test matrix again?

@gp201 Yes, the manual test matrix passed!

@winnie368c winnie368c assigned gp201 and unassigned winnie368c Jun 13, 2023
@winnie368c winnie368c requested a review from gp201 June 13, 2023 20:43
gp201
gp201 approved these changes Jun 23, 2023
View reviewed changes
Copy link
Member

@gp201 gp201 left a comment

Choose a reason for hiding this comment

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

LGTM

@gp201 gp201 requested a review from a team June 23, 2023 07:48
@gp201 gp201 assigned seanlip and unassigned gp201 Jun 23, 2023
@oppiabot oppiabot bot added the PR: LGTM label Jun 23, 2023
@oppiabot
Copy link

oppiabot bot commented Jun 23, 2023

Hi @winnie368c, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

seanlip
seanlip approved these changes Jun 23, 2023
View reviewed changes
Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

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

Thanks @winnie368c!

@seanlip seanlip merged commit fcc2aab into oppia:master Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanlip seanlip seanlip approved these changes

@gp201 gp201 gp201 approved these changes

Assignees

@seanlip seanlip

@winnie368c winnie368c

Labels
PR: LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@winnie368c @SD-13 @gp201 @seanlip