Closed
Bug 1171017
Opened 9 years ago
Closed 9 years ago
Generalize |BluetoothDaemonConnection|
Categories
(Firefox OS Graveyard :: General, defect)
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.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8614705 -
Flags: review?(shuang)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8614706 -
Flags: review?(shuang)
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8614707 -
Flags: review?(shuang)
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8614709 -
Flags: review?(shuang)
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8614710 -
Flags: review?(shuang)
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8614712 -
Flags: review?(shuang)
Assignee | ||
Comment 7•9 years ago
|
||
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.
Assignee | ||
Comment 8•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc6ccecd98a7
Attachment #8614705 -
Flags: review?(shuang) → review+
Assignee | ||
Comment 9•9 years ago
|
||
Changes since v1: - rebased onto latest m-c
Attachment #8614707 -
Attachment is obsolete: true
Attachment #8614707 -
Flags: review?(shuang)
Attachment #8616593 -
Flags: review?(shuang)
Assignee | ||
Comment 10•9 years ago
|
||
Changes since v1: - rebased onto latest m-c
Attachment #8614705 -
Attachment is obsolete: true
Attachment #8617343 -
Flags: review+
Assignee | ||
Comment 11•9 years ago
|
||
Changes since v1: - rebased onto bug 1172479
Attachment #8614706 -
Attachment is obsolete: true
Attachment #8614706 -
Flags: review?(shuang)
Attachment #8617345 -
Flags: review?(shuang)
Assignee | ||
Comment 12•9 years ago
|
||
Changes since v2: - rebased onto bug 1172479
Attachment #8616593 -
Attachment is obsolete: true
Attachment #8616593 -
Flags: review?(shuang)
Attachment #8617347 -
Flags: review?(shuang)
Assignee | ||
Comment 13•9 years ago
|
||
Changes since v2: - [02](v2) didn't build
Attachment #8617345 -
Attachment is obsolete: true
Attachment #8617345 -
Flags: review?(shuang)
Attachment #8617360 -
Flags: review?(shuang)
Assignee | ||
Comment 14•9 years ago
|
||
Changes since v1: - rebased onto bug 1172479
Attachment #8614709 -
Attachment is obsolete: true
Attachment #8614709 -
Flags: review?(shuang)
Attachment #8617370 -
Flags: review?(shuang)
Assignee | ||
Comment 15•9 years ago
|
||
Changes since v1 - rebased onto bug 1172479
Attachment #8614710 -
Attachment is obsolete: true
Attachment #8614710 -
Flags: review?(shuang)
Attachment #8617373 -
Flags: review?(shuang)
Assignee | ||
Comment 16•9 years ago
|
||
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+
Attachment #8617360 -
Flags: review?(shuang) → review+
Attachment #8617370 -
Flags: review?(shuang) → review+
Attachment #8617373 -
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+
Assignee | ||
Comment 19•9 years ago
|
||
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.
Comment 20•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/61079018d0a8 https://hg.mozilla.org/integration/b2g-inbound/rev/529efc4064a1 https://hg.mozilla.org/integration/b2g-inbound/rev/1ad08463f06b https://hg.mozilla.org/integration/b2g-inbound/rev/ffbd7fb52815 https://hg.mozilla.org/integration/b2g-inbound/rev/9d9012f0beda https://hg.mozilla.org/integration/b2g-inbound/rev/69dcab10d33d
Comment 21•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/61079018d0a8 https://hg.mozilla.org/mozilla-central/rev/529efc4064a1 https://hg.mozilla.org/mozilla-central/rev/1ad08463f06b https://hg.mozilla.org/mozilla-central/rev/ffbd7fb52815 https://hg.mozilla.org/mozilla-central/rev/9d9012f0beda https://hg.mozilla.org/mozilla-central/rev/69dcab10d33d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → NGA S3 (26Jun)
Updated•9 years 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.
Description
•