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

Update Onyx hash (LRU cache) #4509

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

kidroca
Copy link
Contributor

@kidroca kidroca commented Aug 9, 2021

Details

Update to use latest Onyx version
Related to PR: Expensify/react-native-onyx#93

Fixed Issues

N/A

Tests

No specific tests. But a general tests for regressions
Init and chat switch performance should be the same or improved slightly

QA Steps

Same as "Tests"

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

Mobile Web

Screenshot 2021-08-09 at 20 10 50

Desktop

Screenshot 2021-08-09 at 20 07 53

iOS

Screenshot 2021-08-09 at 20 06 55

Android

image

@kidroca kidroca requested a review from a team as a code owner August 9, 2021 17:15
@MelvinBot MelvinBot requested review from NikkiWines and removed request for a team August 9, 2021 17:15
"clipboard": "2.0.4",
"html-entities": "^1.3.1",
"jquery": "3.6.0",
"jquery": "3.3.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we're downgrading to an older release of jQuery?

Copy link
Contributor Author

@kidroca kidroca Aug 9, 2021

Choose a reason for hiding this comment

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

It happened automatically and it seem to be the correct version acording to package specification
Onyx uses Expensify/expensify-common@2e5cff5

Probably someone committed incorrect package-lock state

Copy link
Contributor

@NikkiWines NikkiWines Aug 9, 2021

Choose a reason for hiding this comment

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

Ah yeah, it looks like we aren't consistently updating repos with expensify-common dependencies after making changes in expensify-common.

I guess it's fine to merge this as-is... cc: @marcaaron just for awareness. I think we need to be more diligent with the follow-up PRs that Expensify employees should be making after changes to expensify-common.

NikkiWines
NikkiWines previously approved these changes Aug 9, 2021
Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

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

@kidroca you've got some conflicts now sorry but overall looks good

@NikkiWines NikkiWines assigned NikkiWines and kidroca and unassigned NikkiWines Aug 9, 2021
@kidroca
Copy link
Contributor Author

kidroca commented Aug 10, 2021

Fixed conflicts and re-tested
✔️ All good on my end

Copy link
Contributor

@NikkiWines NikkiWines left a comment

Choose a reason for hiding this comment

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

👍

@NikkiWines NikkiWines merged commit 4cebc0f into Expensify:main Aug 10, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @NikkiWines in version: 1.0.83-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 failure ❌
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@botify
Copy link

botify commented Aug 17, 2021

This has been deployed to production and is now subject to a 7-day regression period.
If no regressions arise, payment will be issued on 2021-08-24. 🎊

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @isagoico in version: 1.0.85-9 🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

4 participants