Closed Bug 968671 Opened 6 years ago Closed 6 years ago

Modify Makefile for camera app to migrate camera related code from applications-data.js

Categories

(Firefox OS Graveyard :: Gaia::Build, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: yurenju, Assigned: gduan)

References

Details

Attachments

(1 file)

modify makefile for camera app and migrate camera related code from applications-data.js.
Assignee: nobody → gduan
Attached file PR to master
This patch will be updated again once bug 968680 is merged.
Comment on attachment 8372204 [details] [review]
PR to master

Hi Yuren,
please kindly review it. It should contain gallery and camera parts.
Thanks.
Attachment #8372204 - Flags: review?(yurenju.mozilla)
Comment on attachment 8372204 [details] [review]
PR to master

Hi Diego,
we have migrated some code of build script to gallery and camera app.
Please kindly help to reviewed it, thanks.
Attachment #8372204 - Flags: review?(dmarcos)
Attachment #8372204 - Flags: review?(dmarcos) → review-
Why did you remove apps/camera/build/configure.js? We want to keep it. It's a node script with no dependencies that it's a first step to make the camera application decoupled from the gaia build system. The application will be pulled out eventually to an external repo.
Comment on attachment 8372204 [details] [review]
PR to master

George, the pr looks good but it break executing |make| in camera app directory because we use node to build camera app if xpcshell doesn't exist for now, could you work out a solution for that?
Attachment #8372204 - Flags: review?(yurenju.mozilla)
We should be able to keep existing node dependency to run Camera app Makefile independently. If we need two JS build script in camera/build/ to be able to generate the same file on two environments, so be it. However please come up with a unified API for two environment in the future.
Comment on attachment 8372204 [details] [review]
PR to master

Hi Diego,
Camera should be able to run |make| alone now.

As comment 6 has suggested, I kept two build script for now and will file bug for unified API later.

Please kindly review it again. Thanks.
Attachment #8372204 - Flags: review- → review?(dmarcos)
Comment on attachment 8372204 [details] [review]
PR to master

Yuren should review the Gaia build part.

Let's not put to much load to Diego and ask to review the part he is not familiar.
Attachment #8372204 - Flags: review?(yurenju.mozilla)
Attachment #8372204 - Flags: review?(dmarcos)
Attachment #8372204 - Flags: feedback?(dmarcos)
George, could you file a follow up bug to migrate build/build.js and build/configure.js and add to follow up section of refactoring plan[1]? thanks.

[1] https://wiki.mozilla.org/Gaia/Build/Refactoring_Plan
Comment on attachment 8372204 [details] [review]
PR to master

r=yurenju and please fix nits which mentioend on github.
Attachment #8372204 - Flags: review?(yurenju.mozilla) → review+
Thanks,
merged into master,
https://github.com/mozilla-b2g/gaia/commit/e26bd642be6f7825f8b7e811582bd0e231390317
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 968680
Blocks: 975964
Component: Gaia → Gaia::Build
You need to log in before you can comment on or make changes to this bug.