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

Change Expensify.cash to New Expensify #4408

Merged
merged 19 commits into from
Aug 11, 2021
Merged

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Aug 4, 2021

Details

  1. Change Expensify.cash name to New Expensify.
  2. Change Repo Url to https://github.com/Expensify/App from https://github.com/Expensify/Expensify.cash
  3. Update Readme and Other documentation to show New Expensify.

Fixed Issues

$ Fixes #4213 #4462

Tests | QA Steps

  1. Expensify.cash should not be visible publicly in the App on any platfrom. Instead you should see New Expensify.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Can't be depicted.

@parasharrajat parasharrajat requested a review from a team as a code owner August 4, 2021 14:47
@MelvinBot MelvinBot requested review from Jag96 and removed request for a team August 4, 2021 14:47
@parasharrajat
Copy link
Member Author

@Jag96 Question:

  1. Mac desktop app is packaged as Expensify.cash. Do you want t change it as well and thus the Download url?

@Jag96
Copy link
Contributor

Jag96 commented Aug 4, 2021

@parasharrajat yes let's update that as well, we can use NewExpensify.dmg for the artifactName and use New Expensify as the title

@parasharrajat parasharrajat changed the title [WIP] Change Expensify.cash to New Expensify Change Expensify.cash to New Expensify Aug 5, 2021
@parasharrajat
Copy link
Member Author

@Jag96 I have changed almost all instances. Please check carefully and tested the app by making builds on each platform.

There are still a few places that I am confused of

  1. Detox Config in package.json. I am not sure about the case.
  2. Second Expensify.cash.app
    13B07F961A680F5B00A75B9A /* Expensify.cash.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = Expensify.cash.app; sourceTree = BUILT_PRODUCTS_DIR; };

Could you please help me verify all the changes and tell me if I miss somewhere?
Thanks. It would be a great help.

Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

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

Nice! Looks like you got most of them, here's what I suggest updating to fix the other usages:

  1. Update package.json name to new.expensify, then update the Config.js value
    APP_NAME: 'ExpensifyCash',
  2. Update usages in Timing.js and Session.js https://github.com/Expensify/App/blame/a47c15c9fe5fef0a04105fa6fd0ce01d6e191ef2/src/libs/actions/Session.js#L123 cc @marcaaron since this will likely change some dashboards, not sure if they're being used
  3. Update a couple of comments in desktop/main.js https://github.com/Expensify/App/blob/2e0d58963a92c8c8533758eba86f264e52df8139/desktop/main.js#L37
  4. npm install to update package-lock.json (that should fix the detox config)
  5. Open the Workspace file in Xcode and update the top left folder to be NewExpensify by clicking twice on it and typing (it will ask you to confirm name change after hitting Enter, just confirm)
    image
  6. Rename the folders in Xcode from Expensify.cash to NewExpensify (Same for ExpensifyCashTests)
    image
  7. Go into iOS directory in terminal and remove Podfile.lock, then update the ExpensifyCash name in Podfile to be NewExpensify (where target definitions are), same for tests definition in podfile
  8. Run pod install from iOS Directory
  9. Try building the app on all devices and confirm it works

@parasharrajat
Copy link
Member Author

@Jag96 Renaming the folders breaks a lot of paths in info.plist and other files. How can I autocorrect them? I am new to IOS projects& xcode.

@Jag96
Copy link
Contributor

Jag96 commented Aug 6, 2021

No worries, Xcode is a pain when it comes to this stuff. Can you give an example of a path that is messed up or a screenshot of what you're seeing? I think the main things here are:

  1. Make sure you do step 7 in Change Expensify.cash to New Expensify #4408 (review)
  2. When renaming the folders in Xcode, it is possible that they aren't being renamed in your files, so you may have to right click a folder in Xcode and choose Add files to NewExpensify and re-import them

Going to tag @AndrewGable on this since he did the renaming last time, just to make sure there isn't another strategy that is easier for this

@AndrewGable
Copy link
Contributor

I believe I followed this stack overflow: https://stackoverflow.com/a/35500038/1858217

@parasharrajat
Copy link
Member Author

Thanks, @AndrewGable I got hands-on on that post just today. I am following that.

@parasharrajat
Copy link
Member Author

@Jag96 All set. It was a hack off the task took around 3 days to carefully change and test everything. Xcode is really bad at managing projects. I have to manually change the path for each source file and rename the old ExpensifyCash to NewExpensify in the Xcode Project metadata.

I have tested the build on all platforms.

@parasharrajat
Copy link
Member Author

There is one BasePackageList.java file that is autogenerated every time I run the android build. I think we should include this in the Code.
Path => /android/app/src/main/java/com/expensify/chat/generated/BasePackageList.java
The content of it is as follows

package com.expensify.chat.generated;

import java.util.Arrays;
import java.util.List;
import org.unimodules.core.interfaces.Package;

public class BasePackageList {
  public List<Package> getPackageList() {
    return Arrays.<Package>asList(
        new expo.modules.haptics.HapticsPackage()
    );
  }
}

@parasharrajat
Copy link
Member Author

Update usages in Timing.js and Session.js a47c15c/src/libs/actions/Session.js#L123 (blame) cc since this will likely change some dashboards, not sure if they're being used

I have not done this. This will affect backend Dashboards statistics.

@parasharrajat parasharrajat dismissed stale reviews from Jag96 and Julesssss via 0836f88 August 11, 2021 17:10
@parasharrajat
Copy link
Member Author

@Jag96 Done.

Jag96
Jag96 previously approved these changes Aug 11, 2021
Copy link
Contributor

@Jag96 Jag96 left a comment

Choose a reason for hiding this comment

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

LGTM

@Jag96
Copy link
Contributor

Jag96 commented Aug 11, 2021

#4274 got merged in between, going to post something internally asking everybody to hold off on merging anything so we can get this through. Also, we can skip waiting for tests on this since its just the version bump that is in the diff and they were passing here.

Julesssss
Julesssss previously approved these changes Aug 11, 2021
@parasharrajat
Copy link
Member Author

Enjoy new conflicts.

@Jag96
Copy link
Contributor

Jag96 commented Aug 11, 2021

There's currently cherry-pick testing happening, so we'll have to hold off on updating/merging this until that is done. I'll post back here when we're good to go, and after confirming I'll have everybody hold off on merging until we're done w/ this. Sorry for the back and forth here.

@Jag96
Copy link
Contributor

Jag96 commented Aug 11, 2021

Ok, looks like #4578 didn't work the way I expected it to. CP testing is finished atm so let's try this again. @parasharrajat let me know when you're good to push the conflict resolution commit and I can call for a pause on merging before you push to make sure we get it right this time.

@parasharrajat
Copy link
Member Author

Done @Jag96. Raise the Horn.

@Jag96 Jag96 merged commit d10f6a5 into Expensify:main Aug 11, 2021
@Jag96
Copy link
Contributor

Jag96 commented Aug 11, 2021

Skipping unnecessary actions to get this merged: #4408 (comment)

@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 @Jag96 in version: 1.0.85-10 🚀

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

@isagoico
Copy link

This PR is failing in iOS #4739

@MitchExpensify
Copy link
Contributor

Seeing as #4739 is closed, are we good here @isagoico ?

@isagoico
Copy link

Yep! We can check it off.

@botify
Copy link

botify commented Aug 25, 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-09-01. 🎊

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.0.86-11 🚀

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

@botify
Copy link

botify commented Aug 30, 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-09-06. 🎊

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.

[HOLD for payment + Bonus Aug 18] Change site and app titles to New Expensify
9 participants