Closed Bug 1171017 Opened 4 years ago Closed 4 years ago

Generalize |BluetoothDaemonConnection|

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox41 fixed)

RESOLVED FIXED
FxOS-S1 (26Jun)
Tracking Status
firefox41 --- fixed

People

(Reporter: tzimmermann, Assigned: tzimmermann)

References

Details

Attachments

(6 files, 8 obsolete files)

6.73 KB, patch
tzimmermann
: review+
Details | Diff | Splinter Review
252.12 KB, patch
shawnjohnjr
: review+
Details | Diff | Splinter Review
8.73 KB, patch
shawnjohnjr
: review+
Details | Diff | Splinter Review
5.97 KB, patch
shawnjohnjr
: review+
Details | Diff | Splinter Review
12.56 KB, patch
shawnjohnjr
: review+
Details | Diff | Splinter Review
24.10 KB, patch
shawnjohnjr
: review+
Details | Diff | Splinter Review
|BluetoothDaemonConnection| is useful for many Gonk daemons. We should represent its functionality with something independent from Bluetooth.
I tried to split up this change in a way to make review easy. Patches [01] to [05] only rename classes, one at a time. Patch [06] only moves code around and cleans up the resulting files a bit (includes). There are no changes to the functionality of any of the classes in any of the patches.
Changes since v1:

  - rebased onto latest m-c
Attachment #8614707 - Attachment is obsolete: true
Attachment #8614707 - Flags: review?(shuang)
Attachment #8616593 - Flags: review?(shuang)
Changes since v1:

  - rebased onto latest m-c
Attachment #8614705 - Attachment is obsolete: true
Attachment #8617343 - Flags: review+
Depends on: 1172479
Changes since v1:

  - rebased onto bug 1172479
Attachment #8614706 - Attachment is obsolete: true
Attachment #8614706 - Flags: review?(shuang)
Attachment #8617345 - Flags: review?(shuang)
Changes since v2:

  - rebased onto bug 1172479
Attachment #8616593 - Attachment is obsolete: true
Attachment #8616593 - Flags: review?(shuang)
Attachment #8617347 - Flags: review?(shuang)
Changes since v2:

  - [02](v2) didn't build
Attachment #8617345 - Attachment is obsolete: true
Attachment #8617345 - Flags: review?(shuang)
Attachment #8617360 - Flags: review?(shuang)
Changes since v1:

  - rebased onto bug 1172479
Attachment #8614709 - Attachment is obsolete: true
Attachment #8614709 - Flags: review?(shuang)
Attachment #8617370 - Flags: review?(shuang)
Changes since v1

  - rebased onto bug 1172479
Attachment #8614710 - Attachment is obsolete: true
Attachment #8614710 - Flags: review?(shuang)
Attachment #8617373 - Flags: review?(shuang)
Changes since v1:

  - rebased onto bug 1172479
Attachment #8614712 - Attachment is obsolete: true
Attachment #8614712 - Flags: review?(shuang)
Attachment #8617374 - Flags: review?(shuang)
Comment on attachment 8617347 [details] [diff] [review]
[03] Bug 1171017: Rename |BluetoothSocketPDU| to |DaemonSocketPDU| (v3)

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

It looks good to me.
Attachment #8617347 - Flags: review?(shuang) → review+
Comment on attachment 8617374 [details] [diff] [review]
[06] Bug 1171017: Move classes from ipc/bluetooth to ipc/hal (v2)

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

::: ipc/hal/DaemonSocket.h
@@ +3,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>   * You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#ifndef mozilla_ipc_DaemonSocket_h

nit: It seems other ipc folders use lower case letters, should it be mozilla_ipc_daemonsocket_h

::: ipc/hal/DaemonSocketConsumer.h
@@ +3,5 @@
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>   * You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#ifndef mozilla_ipc_DaemonSocketConsumer_h

nit:ditto, lower case letters.

::: ipc/hal/DaemonSocketPDU.h
@@ +3,5 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef mozilla_ipc_DaemonSocketPDU_h

nit:ditto

::: ipc/hal/moz.build
@@ +11,3 @@
>  ]
>  
> +UNIFIED_SOURCES += [

Thanks for pointing out! We should use UNIFIED_SOURCES
Attachment #8617374 - Flags: review?(shuang) → review+
Hey!

> >  
> > +#ifndef mozilla_ipc_DaemonSocket_h
> 
> nit: It seems other ipc folders use lower case letters, should it be
> mozilla_ipc_daemonsocket_h

On the Coding Style wiki they use capital letters, so I used them as well. I'd say the lower-case ifdefs are the incorrect ones and come from long ago.
Target Milestone: NGA S3 (26Jun) → FxOS-S1 (26Jun)
You need to log in before you can comment on or make changes to this bug.