[DNM] samples: Removal of Zephyr smp_svr sample

[de-nordic]

Zaphyr RTOS is keeping its own copy of sample and this sample has not been updated far too long to work with Zephyr.

Signed-off-by: Dominik Ermel dominik.ermel@nordicsemi.no

[de-nordic]

@carlescufi I think sample in Zephyr is enough.

[carlescufi]

cc @de-nordic and @nvlsianpu

@utzig can you please add @de-nordic as a collaborator to this repo so we can add him as a reviewer?

[utzig]

@utzig can you please add @de-nordic as a collaborator to this repo so we can add him as a reviewer?

I cannot myself, but I opened an issue here: https://issues.apache.org/jira/browse/INFRA-19757

[de-nordic]

I cannot myself, but I opened an issue here: https://issues.apache.org/jira/browse/INFRA-19757

Thank!

[vrahane]

@de-nordic So, what is the plan here, only to maintain a sample in the zephyr repo or the zephyr fork of mcumgr ? Can we add Zephyr's sample here. That way we can actually make sure it works with zephyr going down the road and each time we update the zephyr smp_svr app or we make big changes to mcumgr we should test it with zephyr.

[de-nordic]

@de-nordic So, what is the plan here, only to maintain a sample in the zephyr repo or the zephyr fork of mcumgr ? Can we add Zephyr's sample here. That way we can actually make sure it works with zephyr going down the road and each time we update the zephyr smp_svr app or we make big changes to mcumgr we should test it with zephyr.

Basically I want to avoid duplicate of the same sample, that gets out of sync. I think that this has been your sample, so If you prefer it to be here I do not have really strong arguments to oppose this. Instead of removing this sample, I am willing to replace it with zephyr, updated version and remove it from zephyr instead, though I will need to add some support to make it part of test suite.

To test this with Zephyr you need to use Zephyr anyway.

I need to think it over. I just want to avoid having out of sync versions where someone would have to pick patches by hand and apply them both ways.

[utzig]

@de-nordic @carlescufi So, the answer I got in the ticket was: "This is not possible currently. Write access to ASF repos requires a filed ICLA. This is part of the ASF's legal provenance. If someone is so important to your review process, why would you not invite them as a committer?". So much for doing a review! We can do the process if you don't mind but it takes a few days and you would do sign an ICLA and so on and so forth...

I need to think it over. I just want to avoid having out of sync versions where someone would have to pick patches by hand and apply them both ways.

I am wondering about this as well. Ideally there should be a single copy, but worse case scenario we could at least automate this a bit, I mean something like: "if someone touches this code under smp_svr show a message in the CI saying that it needs to be sent upstream". That can be done on the Zephyr CI correct? We could also add it here, it's currently lacking travis integration but other Mynewt repos already do have it so it's just a matter of applying it here as well...

[de-nordic]

I am wondering about this as well. Ideally there should be a single copy, but worse case scenario we could at least automate this a bit, I mean something like: "if someone touches this code under smp_svr show a message in the CI saying that it needs to be sent upstream".

I think it could be done. Probably people involved in reviewing changes to smp_svr would also others on the need to update.

[mlaz]

I am wondering about this as well. Ideally there should be a single copy, but worse case scenario we could at least automate this a bit, I mean something like: "if someone touches this code under smp_svr show a message in the CI saying that it needs to be sent upstream".

I am with @utzig on this one.

[vrahane]

So, the problem with having one single copy is, the upstream and the forked version of mcumgr can be out of sync. I feel we should maintain both just so that the forked version always has its own working copy and the upstream has its own working copy or there is sort of an issue here where it might work either on the upstream or the forked version but not both.

[de-nordic]

Lets abandon the idea about removing the sample and lets update it instead: https://github.com/apache/mynewt-mcumgr/pull/62

[nvlsianpu]

So, the problem with having one single copy is, the upstream and the forked version of mcumgr can be out of sync. I feel we should maintain both just so that the forked version always has its own working copy and the upstream has its own working copy or there is sort of an issue here where it might work either on the upstream or the forked version but not both.

zephyr-rtos uses exact SHA for reference mcumgr - doesn't this help?

[carlescufi]

I don't understand. This sample is maintained inside the zephyr tree. What is exactly the point of maintaining this copy here @de-nordic?

[de-nordic]

As I understand @vrahane , they want to keep their copy, so I am at least updating it for them.

[carlescufi]

As I understand @vrahane , they want to keep their copy, so I am at least updating it for them.

@vrahane I disagree with keeping this copy here. What should be done is integrate CI with building the sample against Zephyr or manually testing PRs against Zephyr (and Mynewt of course)

[vrahane]

@carlescufi In general, with all the sub repos, we maintain the samples in the repo, in this case it is important because there is some zephyr specific code in there which needs to be tested each time the library changes for example this time when we were updating it, it broke a few things on the zephyr side which needed testing. Moreover, the changes in mcumgr can vary in the fork and the upstream. I think both should maintain their own version, now if the fork was not there, it would have made sense. We do a similar thing for mcuboot where the zephyr sample gets maintained along with the mynewt sample. I would like this to be in sync.

[carlescufi]

@carlescufi In general, with all the sub repos, we maintain the samples in the repo, in this case it is important because there is some zephyr specific code in there which needs to be tested each time the library changes for example this time when we were updating it, it broke a few things on the zephyr side which needed testing. Moreover, the changes in mcumgr can vary in the fork and the upstream. I think both should maintain their own version, now if the fork was not there, it would have made sense. We do a similar thing for mcuboot where the zephyr sample gets maintained along with the mynewt sample. I would like this to be in sync.

But the thing is that you are not going to be able to test this sample unless it's tied to a particular revision of Zephyr. Instead, in the case of Zephyr, we keep a reference to a stable version of mcumgr, which in my opinion is the correct dependency order. @utzig what revision of Zephyr are you using in #63?

[utzig]

@utzig what revision of Zephyr are you using in #63?

Currently master.

[carlescufi]

@utzig what revision of Zephyr are you using in #63?

Currently master.

Right, I don't think that's gonna work reliably. There are API deprecations and changes in zephyr periodically, so I do believe that the better approach is the opposite. I will not block the PR, but I am not convinced that it provides much value.

[utzig]

so I do believe that the better approach is the opposite.

What do you mean by "opposite"?

[nvlsianpu]

What do you mean by "opposite"

Exactly keeping smp_svr exclusively in the zephyr code-base, which force us, so zephyr community to maintenance it. so #60 approach

[de-nordic]

No longer valid after https://github.com/apache/mynewt-mcumgr/pull/62 has been merged.