apache / mynewt-mcumgr

Apache mynewt
https://mynewt.apache.org/
99 stars 76 forks source link

cmd/os_mgmt/port/zephyr: zephyr port os enhancement #81

Closed lgl88911 closed 4 years ago

lgl88911 commented 4 years ago

This patch add two new features:

Signed-off-by: Frank Li lgl88911@163.com

de-nordic commented 4 years ago

Can you point me to where you are reviewing Zephyr west.yml (https://github.com/zephyrproject-rtos/zephyr/pulls) changes for this?

utzig commented 4 years ago

Can you point me to where you are reviewing Zephyr west.yml (https://github.com/zephyrproject-rtos/zephyr/pulls) changes for this?

@lgl88911 Do you mind to comment?

lgl88911 commented 4 years ago

@de-nordic @utzig Sorry,I am relatively new to this process, can you elaborate on the process? Or is there a document for reference?

de-nordic commented 4 years ago

@lgl88911 General contribution guidelines are here https://docs.zephyrproject.org/latest/contribute/index.html?highlight=contributing, but unfortunately you have crossed into the part that have not yet been written (or at least I could not find it).

So: you have done Zephyr specific code that is not within zephyr os repo and yet you still need to have CI green before merging that, that is why you need to create Zephryproject-rtos/zephyr PR that will test it for you. And the other thing is that when someone wants to test it, it is enough for the person to just checkout your Zephyr PR and invoke west to configure all dependent repos same way you had them configured when creating PR (makes things easier).

First rebase your zephyrproject-rtos/zephyr to newest master and west update.

As an example, here is PR that tries to review mcuboot changes: https://github.com/zephyrproject-rtos/zephyr/pull/25696

Notice that Torsten had modified west.yml file to point out to his pull reqest, to mcuboot. In you case you will have to add additional information for remotes (because this repo is outside of zephyrproject):

- name: apache
   url-base: https://github.com/apache/

And where you have mcumgr in west, change name from mcumgr to mynewt-mcumgr (same way the repo is called) and the revision to your pull request pull/81/head.

No warranties that he zephyrprojet-rtos/Zephyr PR will be merged, because if the review passes (here), there is a chance that some other PR will update west beyond your commit here.

de-nordic commented 4 years ago

Got informed that documentation is here: https://docs.zephyrproject.org/latest/guides/modules.html#changes-to-existing-modules so I am hiding my previous comment that describes the process.

lgl88911 commented 4 years ago

Got informed that documentation is here: https://docs.zephyrproject.org/latest/guides/modules.html#changes-to-existing-modules so I am hiding my previous comment that describes the process.

Thank you for your guidance, I am not sure if this is correct, please help check this pr https://github.com/zephyrproject-rtos/zephyr/pull/25711

de-nordic commented 4 years ago

@utzig Thanks for merging it.