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

chore(devDeps): upgrade from metro 0.71 to 0.73 #7640

Merged
merged 16 commits into from Jan 18, 2024

Conversation

legobeat
Copy link
Contributor

@legobeat legobeat commented Oct 30, 2023

Description

Context.

Currently, three different versions of metro packages are being pulled in. Specifically, the explicitly depended on version is not even used.

This removes duplicates and aligns on the highest currently used version, 0.73.10.

This implies a bump of metro-config to the matching version.

Related issues

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Oct 30, 2023
@socket-security
Copy link

socket-security bot commented Oct 30, 2023

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: metro-config@0.71.1

@socket-security
Copy link

socket-security bot commented Oct 30, 2023

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (eb51857) 39.83% compared to head (f507355) 39.83%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7640   +/-   ##
=======================================
  Coverage   39.83%   39.83%           
=======================================
  Files        1233     1233           
  Lines       29821    29821           
  Branches     2840     2840           
=======================================
  Hits        11880    11880           
  Misses      17252    17252           
  Partials      689      689           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@legobeat
Copy link
Contributor Author

@SocketSecurity ignore node-idevice@0.1.6

new author ok; unrelated and not sure why it's showing up in all open PRs now

@legobeat legobeat changed the title devDeps: upgrade from metro 0.71 to 0.73 chore(devDeps): upgrade from metro 0.71 to 0.73 Oct 30, 2023
@legobeat legobeat added dependencies Pull requests that update a dependency file team-security labels Oct 30, 2023
@legobeat legobeat marked this pull request as ready for review October 30, 2023 01:32
@legobeat legobeat requested a review from a team as a code owner October 30, 2023 01:32
@legobeat
Copy link
Contributor Author

Hmm, could this CI error be unrelated?

$ ts-node ./.github/scripts/check-bitrise-e2e-smoke/check-e2e-smoke-trigger.ts
Workflow triggered by the event (ready_for_review)
Starting E2E smoke since PR has changed to ready for review.
Error occured when applying label: HttpError: Resource not accessible by integration
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

@cortisiko
Copy link
Member

Hmm, could this CI error be unrelated?

@legobeat
Triggering the E2E tests automatically is specifically supported on branches but not on forks. Can you open a branch inside the mobile repo instead of creating a fork branch?

@leotm
Copy link
Contributor

leotm commented Oct 30, 2023

great explanation again earlier

re-thinking let's go couple last steps further

  • move metro-config from dep to devDep
  • (more thought) untrack it in package.json

so now fully/officially aligned on RN core w simplest setup possible

then cleanup

@legobeat
Copy link
Contributor Author

great explanation again earlier

re-thinking let's go couple last steps further

* move `metro-config` from _dep_ to _devDep_

* (more thought) untrack it in package.json

so now fully/officially aligned on RN core w simplest setup possible

then cleanup

I do think metro-config could stay as a devDependency though - as it is imported explicitly in metro.config.js.

But thanks for reminding me on .depcheckrc.yml - updated.

package.json Outdated Show resolved Hide resolved
leotm
leotm previously approved these changes Oct 30, 2023
Copy link
Contributor

@leotm leotm left a comment

Choose a reason for hiding this comment

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

🧹🚀

leotm
leotm previously approved these changes Oct 30, 2023
Copy link
Contributor

@leotm leotm left a comment

Choose a reason for hiding this comment

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

ci / dupe in action 👌

worth adding to our husky step?

"husky": {
"hooks": {
"pre-commit": "lint-staged"
}
},

edit: actually prob not, running this per commit would get annoying

@legobeat legobeat force-pushed the devDeps-metro-0.73 branch 2 times, most recently from fccdd9e to d43e582 Compare October 30, 2023 23:58
@legobeat legobeat requested review from a team and leotm October 30, 2023 23:58
used version is already pulled in via metro-config - this added an
extraneous duplicate version
@legobeat legobeat requested a review from leotm November 20, 2023 01:41
@Cal-L
Copy link
Contributor

Cal-L commented Nov 20, 2023

Do you mind providing a video of the app using the latest version of metro

@legobeat
Copy link
Contributor Author

legobeat commented Nov 20, 2023

Do you mind providing a video of the app using the latest version of metro

I don't have working setup to do that locally right now, unfortunately. Could you help me with this?

@Cal-L
Copy link
Contributor

Cal-L commented Nov 21, 2023

Confirmed working -
image

@Cal-L
Copy link
Contributor

Cal-L commented Nov 21, 2023

started E2E smoke tests - https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/bf829352-ad45-4897-8be8-0a906a441e88

@Cal-L Cal-L added the regression-RC This is placeholder label for QA Regression testing that would be used to flag QA regression work. label Nov 21, 2023
@Cal-L
Copy link
Contributor

Cal-L commented Nov 21, 2023

Could you fix the invalid PR template check?

@legobeat legobeat removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Nov 21, 2023
@legobeat legobeat requested review from a team and removed request for a team November 29, 2023 11:13
@legobeat
Copy link
Contributor Author

Could you fix the invalid PR template check?

I removed the label, does that suffice? Couldn't make out anything more actionable from the error message.

@legobeat legobeat requested a review from Cal-L November 29, 2023 11:15
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Nov 29, 2023
@legobeat legobeat added Run Smoke E2E and removed INVALID-PR-TEMPLATE PR's body doesn't match template labels Jan 15, 2024
@legobeat legobeat merged commit a15b2ea into MetaMask:main Jan 18, 2024
25 of 26 checks passed
@legobeat legobeat deleted the devDeps-metro-0.73 branch January 18, 2024 10:01
@github-actions github-actions bot locked and limited conversation to collaborators Jan 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Pull requests that update a dependency file regression-RC This is placeholder label for QA Regression testing that would be used to flag QA regression work. team-security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants