Last Comment Bug 735017 - Clean up namespaces and naming for gonk dom objects in gonk specific code
: Clean up namespaces and naming for gonk dom objects in gonk specific code
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: All Gonk (Firefox OS)
: -- normal (vote)
: mozilla14
Assigned To: Kyle Machulis [:kmachulis] [:qdot]
:
Mentors:
Depends on:
Blocks: b2g-ril b2g-bluetooth
  Show dependency treegraph
 
Reported: 2012-03-12 13:38 PDT by Kyle Machulis [:kmachulis] [:qdot]
Modified: 2012-03-15 08:21 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch 1: Change dom/system/b2g to dom/system/gonk (489.35 KB, patch)
2012-03-12 15:01 PDT, Kyle Machulis [:kmachulis] [:qdot]
philipp: review-
Details | Diff | Review
Patch 2: Change mozilla::dom::telephony to mozilla::dom::gonk for dom/system/gonk objects (3.65 KB, patch)
2012-03-12 15:23 PDT, Kyle Machulis [:kmachulis] [:qdot]
bent.mozilla: review+
philipp: feedback+
Details | Diff | Review
Patch 1: Change dom/system/b2g to dom/system/gonk (6.79 KB, patch)
2012-03-14 14:40 PDT, Kyle Machulis [:kmachulis] [:qdot]
philipp: review+
Details | Diff | Review
Patch 1: Change dom/system/b2g to dom/system/gonk (6.79 KB, patch)
2012-03-14 14:43 PDT, Kyle Machulis [:kmachulis] [:qdot]
no flags Details | Diff | Review

Description Kyle Machulis [:kmachulis] [:qdot] 2012-03-12 13:38:48 PDT
Due to the fact that telephony was the first radio work we did, everything involving radios (phone, wifi, soon to be bt/nfc) is actually under the mozilla::dom::telephony namespace. We should rename these to be more general, and set up defines per radio type plus an overall MOZ_GONK_RADIOS (true whenever any of MOZ_GONK_BLUETOOTH/MOZ_GONK_WIFI/MOZ_GONK_RIL/MOZ_GONK_NFC are defined) to make configuration in builds easier.
Comment 1 Kyle Machulis [:kmachulis] [:qdot] 2012-03-12 15:01:47 PDT
Created attachment 605146 [details] [diff] [review]
Patch 1: Change dom/system/b2g to dom/system/gonk

First patch to rename dom/system/b2g to dom/system/gonk
Comment 2 Kyle Machulis [:kmachulis] [:qdot] 2012-03-12 15:23:56 PDT
Created attachment 605160 [details] [diff] [review]
Patch 2: Change mozilla::dom::telephony to mozilla::dom::gonk for dom/system/gonk objects

Change namespaces to be more general. Also changed the title, since this may not be just radios at some point, but rather mobile devices we need to support otherwise (non-host-side USB for instance)
Comment 3 Philipp von Weitershausen [:philikon] 2012-03-12 16:23:03 PDT
Comment on attachment 605160 [details] [diff] [review]
Patch 2: Change mozilla::dom::telephony to mozilla::dom::gonk for dom/system/gonk objects

This is ok with me, but I don't know our C++ namespacing conventions well enough. Paging Dr Turner.
Comment 4 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-03-13 15:45:31 PDT
Comment on attachment 605160 [details] [diff] [review]
Patch 2: Change mozilla::dom::telephony to mozilla::dom::gonk for dom/system/gonk objects

Chatted with jonas about this a bit, we don't like that everything is somehow under dom::telephony, and we think that is the real problem. We'd prefer telelphony stuff to live in dom::telephony, sms stuff to be in dom::sms, wifi in dom::wifi, etc. Moving everything to dom::gonk is not all that useful in our opinion (though SystemWorkerManager could probably go there).
Comment 5 Kyle Machulis [:kmachulis] [:qdot] 2012-03-13 17:15:49 PDT
I think you're misreading the bug. The only parts I changed to be mozilla::dom::gonk are the SystemWorker and AudioManager references, which are most definitely gonk only, and were under telephony before which didn't make sense. Everything else will stay in their own namespaces.
Comment 6 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-03-14 12:42:50 PDT
Comment on attachment 605160 [details] [diff] [review]
Patch 2: Change mozilla::dom::telephony to mozilla::dom::gonk for dom/system/gonk objects

Review of attachment 605160 [details] [diff] [review]:
-----------------------------------------------------------------

Ah! Right. Sorry about that!
Comment 7 Philipp von Weitershausen [:philikon] 2012-03-14 14:25:07 PDT
Comment on attachment 605146 [details] [diff] [review]
Patch 1: Change dom/system/b2g to dom/system/gonk

This is good stuff, but the patch should use `hg mv` to rename the files.
Comment 8 Kyle Machulis [:kmachulis] [:qdot] 2012-03-14 14:40:54 PDT
Created attachment 605958 [details] [diff] [review]
Patch 1: Change dom/system/b2g to dom/system/gonk

Redone with hg mv.
Comment 9 Kyle Machulis [:kmachulis] [:qdot] 2012-03-14 14:43:03 PDT
Created attachment 605960 [details] [diff] [review]
Patch 1: Change dom/system/b2g to dom/system/gonk
Comment 10 Kyle Machulis [:kmachulis] [:qdot] 2012-03-14 14:43:45 PDT
Comment on attachment 605960 [details] [diff] [review]
Patch 1: Change dom/system/b2g to dom/system/gonk

Review of attachment 605960 [details] [diff] [review]:
-----------------------------------------------------------------

Ignore. Accidental add.
Comment 11 Philipp von Weitershausen [:philikon] 2012-03-14 14:47:03 PDT
Comment on attachment 605958 [details] [diff] [review]
Patch 1: Change dom/system/b2g to dom/system/gonk

Review of attachment 605958 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the following things addressed:

::: dom/src/geolocation/Makefile.in
@@ +84,1 @@
>                     $(NULL)

While you're in there you can also get rid of the $(NULL) and backslash. Not needed for a single line.

::: dom/system/b2g/Makefile.in
@@ +12,5 @@
>  include $(DEPTH)/config/autoconf.mk
>  
>  MODULE           = dom
> +LIBRARY_NAME     = domsystemgonk_s
> +XPIDL_MODULE     = dom_system_gonk

Changing the XPIDL module name means you also have to update the package manifests in b2g/installer/ and browser/installer/

Note You need to log in before you can comment on or make changes to this bug.