Directory structure adr #15
Conversation
pan-
left a comment
pan-
left a comment
There was a problem hiding this comment.
Thanks Paul, what do you think of making the inclusion pattern consistent with Mbed OS ?
doc/adr/0003-directory-structure.md
Outdated
| An example service called `example` would look like this: | ||
|
|
||
| ``` | ||
| services/ble-service-example/include/example.h |
There was a problem hiding this comment.
That's not in line with what's in mbed-os at the moment which is include/library-name to include the header files with library-name/header.h.
doc/adr/0003-directory-structure.md
Outdated
| @@ -0,0 +1,39 @@ | |||
| # 3. Define initial testing infrastructure | |||
There was a problem hiding this comment.
Please update the name of this ADR.
|
You forgot to change the include structure in 003fe26 |
|
Sorry, it's the constant timeouts yesterday (it messes up github desktop). |
|
@AGlass0fMilk Are you happy with the proposed structure ? |
|
I thought the idea was that once the service stops being experimental it can be moved to mbed-os. Is that not the case? |
It is all about the level of support and integration. Given the trend I think it would not be moved to Mbed OS. @AGlass0fMilk It would be great to have reusable utilities/helper classes shared across the services of the repo. |
pan-
left a comment
pan-
left a comment
There was a problem hiding this comment.
Forget about my last comment, this is the ADR for the directory structure.
I added a proposal for extensions structure. Let me know what you think.
| so it's easy for the user to pick up any one service. | ||
|
|
||
| ## Decision | ||
|
|
There was a problem hiding this comment.
I think we can split the Decision in several parts: services and extensions. Maybe we could mention test that goes into tests. I don't think there is a need to detail it at the moment.
For extensions, what do you think of having them in an extensions folder organized like Mbed OS BLE ?
extensions/include/ble/gattextensions/include/ble/gapextensions/include/ble/commonextensions/source/...
That way, include path shouldn't change if these extensions are included into Mbed OS.
There was a problem hiding this comment.
sounds good, might as well put this into this adr
paul-szczepanek-arm
force-pushed
the
main
branch
from
0e7180a to
ed1b06a
Compare
pan-
left a comment
pan-
left a comment
There was a problem hiding this comment.
Thanks @paul-szczepanek-arm it is good to have more details.
|
@AGlass0fMilk Are you satisfied with the updates made to the document ? |
3 participants
We need to agree on this so we don't clash. Please comment on the proposal.