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

[NO QA] Add YAPL build step to HybridApp builds #55426

Closed
wants to merge 4 commits into from

Conversation

Julesssss
Copy link
Contributor

@Julesssss Julesssss commented Jan 17, 2025

Explanation of Change

Engineers building the Android HybridApp for the first time see a failure due to yapl code not being built. They they need to search slack and manually do this which is confusing and time-consuming. This change ensures it is done automatically.

Then I deleted step 3 from the HybridApp build instructions.

I should be able to remove the manual step in our build workflows too, but I'll do that separately.

Fixed Issues

#54877

Tests

A) Script

  • grunt build: shared should run when an Android build is started
  • Screenshot 2025-01-17 at 11 10 23

B) Build

  • Clone a fresh App repo
  • Clone the Mobile-Expensify submodule
  • Build the android app
    • cd app
    • git submodule update --init --progress --depth 100
    • npm i
    • npm run android
  • It should build, and the app should not crash on start!

@Julesssss Julesssss self-assigned this Jan 17, 2025
Copy link

melvin-bot bot commented Jan 17, 2025

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@Julesssss Julesssss changed the title Add YAPL build step to HybridApp android builds [NO QA] Add YAPL build step to HybridApp android builds Jan 17, 2025
@Julesssss Julesssss changed the title [NO QA] Add YAPL build step to HybridApp android builds [NO QA] Add YAPL build step to HybridApp builds Jan 17, 2025
@Julesssss Julesssss force-pushed the jules-addYAPLBuildStep branch from 674845c to 4a4faec Compare January 17, 2025 19:06
@Julesssss Julesssss requested review from AndrewGable and a team January 17, 2025 19:17
@melvin-bot melvin-bot bot requested review from ahmedGaber93 and removed request for a team January 17, 2025 19:17
@Julesssss Julesssss removed the request for review from ahmedGaber93 January 17, 2025 19:18
@Expensify Expensify deleted a comment from melvin-bot bot Jan 17, 2025
@Julesssss
Copy link
Contributor Author

I dm'd Mariusz for a review too. I'm not sure if a separate script is overkill or good practice for keeping these scripts tidy. Let me know what you think!

@Julesssss Julesssss marked this pull request as ready for review January 17, 2025 19:21
@Julesssss Julesssss requested a review from a team as a code owner January 17, 2025 19:21
@melvin-bot melvin-bot bot removed the request for review from a team January 17, 2025 19:21
Copy link

melvin-bot bot commented Jan 17, 2025

@ Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@Julesssss Julesssss requested a review from a team January 17, 2025 19:49
@melvin-bot melvin-bot bot removed the request for review from a team January 17, 2025 19:49
Copy link

melvin-bot bot commented Jan 17, 2025

@ Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@Julesssss
Copy link
Contributor Author

Replacing these App changes with a better Gradle fix here: https://github.com/Expensify/Mobile-Expensify/pull/13376

@Julesssss Julesssss closed this Jan 21, 2025
@Julesssss Julesssss deleted the jules-addYAPLBuildStep branch January 21, 2025 22:21
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.

3 participants