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

Remove internationalization beta #4967

Merged
merged 3 commits into from
Sep 1, 2021

Conversation

roryabraham
Copy link
Contributor

Details

The locale picker cannot be used from the signup or login pages because betas are not yet available (because the user is not logged in). Dropped a message in Slack here, but I feel like at this point we can safely remove this beta entirely.

Fixed Issues

$ #4958

Note: The above issue is not reproducible on dev, because all betas are enabled by default.

Tests / QA Steps

  1. Go to signin page.
  2. There should be a locale picker in the bottom of the signin page. Changing it should change the language.
  3. Go to the password page.
  4. There should be a locale picker in the bottom of the password page. Changing it should change the language.

Screenshots

Not including screenshots because it looks the same on dev as it did before – this change will only be visible on staging.

@roryabraham roryabraham self-assigned this Aug 31, 2021
@roryabraham roryabraham requested a review from a team as a code owner August 31, 2021 23:17
@github-actions
Copy link
Contributor

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@MelvinBot MelvinBot requested review from aldo-expensify and removed request for a team August 31, 2021 23:17
@roryabraham
Copy link
Contributor Author

cc @iwiznia Because internationalization was originally your project I believe.

aldo-expensify
aldo-expensify previously approved these changes Aug 31, 2021
Copy link
Contributor

@aldo-expensify aldo-expensify left a comment

Choose a reason for hiding this comment

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

The changes look good to me. Tested in web and Android.

We may want to create a follow up GH issue to keep the chosen language once we login or signup.

@roryabraham
Copy link
Contributor Author

We may want to create a follow up GH issue to keep the chosen language once we login or signup.

Reported in #expensify-open-source 👍

@roryabraham
Copy link
Contributor Author

Merging to resolve deploy blocker

@roryabraham roryabraham merged commit 9a06c42 into main Sep 1, 2021
@roryabraham roryabraham deleted the Rory-RemoveInternationalizationBeta branch September 1, 2021 00:00
github-actions bot pushed a commit that referenced this pull request Sep 1, 2021
…onBeta

Remove internationalization beta

(cherry picked from commit 9a06c42)
@OSBotify
Copy link
Contributor

OSBotify commented Sep 1, 2021

🚀 Cherry-picked to staging by @roryabraham in version: 1.0.90-2 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@OSBotify
Copy link
Contributor

OSBotify commented Sep 1, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Sep 1, 2021

🚀 Deployed to staging by @roryabraham in version: 1.0.90-3 🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Sep 2, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.91-0 🚀

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

@iwiznia
Copy link
Contributor

iwiznia commented Sep 13, 2021

Oh crap! I was OOO and did not catch this. We should not remove the beta, the internationalization project is still one, given we only have one language and that we have not internationalized the server (and thus a lot of the messaging people see). Remember we only added internationalization early in the App so that it would be easier to internationalize later.
Can we revert this and re-add the beta please?

@Jag96
Copy link
Contributor

Jag96 commented Sep 14, 2021

@iwiznia thanks for the context, I'll put up a revert PR for this

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.

5 participants