[NO QA] Add YAPL build step to HybridApp builds #55426
Conversation
|
npm has a |
Julesssss
changed the title
Julesssss
changed the title
Julesssss
force-pushed
the
jules-addYAPLBuildStep
branch
from
674845c to
4a4faec
Compare
melvin-bot
bot
requested review from
ahmedGaber93
and removed request for
a team
|
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! |
|
@ 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] |
|
@ 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] |
| @@ -15,7 +15,7 @@ | |||
| "postinstall": "./scripts/postInstall.sh", | |||
| "clean": "./scripts/clean.sh", | |||
| "clean-standalone": "STANDALONE_NEW_DOT=true ./scripts/clean.sh", | |||
| "android": "./scripts/set-pusher-suffix.sh && ./scripts/run-build.sh --android", | |||
| "android": "./scripts/set-pusher-suffix.sh && cd Mobile-Expensify && npm run grunt:build:shared && cd .. && ./scripts/run-build.sh --android", | |||
There was a problem hiding this comment.
Don't we need to run this on iOS too?
There was a problem hiding this comment.
I'm not sure. I saw this iOS build step which made me think no... and we only added the step to the Android workflows when our builds were crashing. Not 100% sure though
There was a problem hiding this comment.
I think this should be running via grunt, but for whatever reason it's not working and engineers miss the error.
There was a problem hiding this comment.
There was a problem hiding this comment.
Oooh that would be better. Perhaps our current grade task name no longer matches this logic:
theTask.name.matches("(assemble|bundle).*(Debug|Adhoc|Release)")
There was a problem hiding this comment.
I'll look into this further when I have time next week
There was a problem hiding this comment.
I think that cleanest solution would be to indeed to include the task in the gradle script. Now I think there is a chance that the React Native CLI may interfere into the build phases, or indeed change it in some way so the buildGrunt doesn't trigger.
Maybe changing the pattern to (install|assemble|bundle).*(Debug|Adhoc|Release) would be enough?
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: sharedshould run when an Android build is startedB) Build
cd appgit submodule update --init --progress --depth 100npm inpm run android