Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nanostack release v704 #5602

Merged
merged 11 commits into from
Nov 30, 2017
Merged

Nanostack release v704 #5602

merged 11 commits into from
Nov 30, 2017

Conversation

artokin
Copy link
Contributor

@artokin artokin commented Nov 28, 2017

Description

Nanostack v7.0.4 release for mbed-os-5.7

-Apache license added to files that were missing it
-Obsolete files removed

Libraries updated:

  • features/nanostack/FEATURE_NANOSTACK/sal-stack-nanostack
  • features/nanostack/FEATURE_NANOSTACK/coap_service

Status

READY

Arto Kinnunen added 6 commits November 28, 2017 09:45
* commit '6887e495f0cb0b3009e4da7c0282c1542bbb2608':
  Squashed 'features/nanostack/FEATURE_NANOSTACK/sal-stack-nanostack/' changes from 0a5ef1c..0697d9a
…changes from 0a5ef1c..0697d9a

0697d9a Remove Jenkinsfile and .gitmodules
f983190 Merge branch 'release_internal' into release_external
86c0f98 Nanostack v7.0.3 release corrections (ARMmbed#1491)

git-subtree-dir: features/nanostack/FEATURE_NANOSTACK/sal-stack-nanostack
git-subtree-split: 0697d9a
… from b1c9efb..471121d

471121d Update apache license (ARMmbed#84)
7cb9f65 Enable COAP traces in Linux builds (ARMmbed#82)

git-subtree-dir: features/nanostack/FEATURE_NANOSTACK/coap-service
git-subtree-split: 471121d
* commit 'f5265c3d2083333276909cf7b16c89657aa7d58a':
  Squashed 'features/nanostack/FEATURE_NANOSTACK/coap-service/' changes from b1c9efb..471121d
@0xc0170
Copy link
Contributor

0xc0170 commented Nov 28, 2017

@artokin Thanks, we will review it shortly

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 28, 2017

One question - https://github.com/ARMmbed/mbed-os/tree/70e7b40468854d33431889a9cd415364c00a2501/features/nanostack/FEATURE_NANOSTACK/sal-stack-nanostack - all the files in the root are relevant (makefiles, scripts, .git files)

Copy link
Contributor

@adbridge adbridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@artokin
The following points need addressing before this is considered suitable for 5.7 release:

  1. Why is the structure:
    features/nanostack/FEATURE_NANOSTACK

    instead of just
    features/FEATURE_NANOSTACK ??

  2. clone_nanostack.sh

    Should this be in the mbed-os repo? If so should it have a licence header ?

  3. coap_service

    a) Should the .sh, makefile etc files have licence headers ? Should these files even be here ?
    b) How are the tests compiled and run? If these are to remain here and public then there should be documentation on how to run them ...

  4. /mbed-mesh-api/test/6lowpan_nd

    This mentions yotta as a requirement - is this still correct? This is deprecated from mbed-os so is going to send mixed messages? Do these test instructions actually work when run from mbed-os clone ?

  5. sal-stack-nanostack

    Also has a huge set of unit tests in two different folders. Do we really want them here and if so there needs to be instructions on running them and licence headers in files accordingly...

General comment: We are not happy to take all the test code as it currently stands, if you do not have time to bring all that up to scratch including the running instructions prior to Thursday's release deadline, can i suggest that all the test code is removed ? It can always be added again at a later date when ready..

@artokin
Copy link
Contributor Author

artokin commented Nov 28, 2017

@0xc0170, Makefile and scripts are needed to execute unit tests. git-files are there like in the other "git-subtree" released components (coap-service, mbed-trace, etc).

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 28, 2017

Makefile and scripts are needed to execute unit tests

OK then if we are keeping them, have similar question to Anna, will wait for the answer to those.

@artokin
Copy link
Contributor Author

artokin commented Nov 28, 2017

@adbridge, thanks for the comments.

  1. The structure is features/nanostack/FEATURE_NANOSTACK as we replaced sal-stack-nanostack binary content with source content. No component restructuring was made.

  2. clone_nanostack.sh is obsolete and it is removed

  3. coap_service has been part of mbed-os quite a long time. We haven't restructured this folder now.
    a) We can add license headers to makefiles and scripts. They are needed for executing unit tests. I'll make a fix commit for this.
    b) test execution instructions are needed (TBD)

  4. /mbed-mesh-api/test/6lowpan_nd was not updated in this PR. It contains old instructions that needs to be cleaned. Will be solved in ONME-3307

  5. sal-stack-nanostack contains similar unit tests like coap-service. Test execution instructions (TBD) are missing just like in the coap-service.

@mikter and @TuomoHautamaki, how do we proceed with the unit tests? Any suggestions?

@SeppoTakalo
Copy link
Contributor

I would actually propose not to modify any of those Makefiles, etc. that are from original repository.
I the future we are planning to update Nanostack by using git subtree --pull so any local modifications can ruin it or make merging harder.

If this is not acceptable, then probably all the Makefiles and unittests have to be gone. Makes no sense to keep Makefiles just for unittests because no modifications to stack would be accepted here. All testing is to be done against the master copy of Nanostack's repository.

Therefore, either remove all that is not relevant to Mbed OS, or keep structure intact for purpose of helping merges.

Arto Kinnunen added 2 commits November 29, 2017 09:41
… from 471121d..29bfb78

29bfb78 Add apache license to Makefiles and scripts (ARMmbed#85)

git-subtree-dir: features/nanostack/FEATURE_NANOSTACK/coap-service
git-subtree-split: 29bfb78
…k_release_v704

* commit 'f900accac5b4efc426e0c2889e78ed9cd8de9b68':
  Squashed 'features/nanostack/FEATURE_NANOSTACK/coap-service/' changes from 471121d..29bfb78
@artokin
Copy link
Contributor Author

artokin commented Nov 29, 2017

Previous commit will fix item 3a, license headers added to makefiles and scripts in coap-service

@TuomoHautamaki
Copy link

Please note that there are already released components including similar unit tests in mbed OS which need the same guidelines (coap-service, libservice, mbed-client-randlib).

In short term enabling release testing and helping merges seen more crucial than temporarily removing nanostack tests due to missing guidelines.

Fully agree the need to have proper documentation for the unit tests. We will take action to solve this issue with coap-service, libservice, mbed-client-randlib and nanostack libraries but it will not be ready for mbed OS 5.7.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 29, 2017

Fully agree the need to have proper documentation for the unit tests. We will take action to solve this issue with coap-service, libservice, mbed-client-randlib and nanostack libraries but it will not be ready for mbed OS 5.7.

Understood, thus our proposal is as below.

Therefore, either remove all that is not relevant to Mbed OS, or keep structure intact for purpose of helping merges.

@SeppoTakalo +1.
We should not bring more tests that are not active, no documentation. We should keep them out from here until they are functional, tested, and have documentation. This will come as new pull request. This should happen today to get this patch ready. If any more questions , lets discuss it.

The rest of the tests in the repository will be taken care of separately. This scope is for nanostack, so lets focus about nanostack and its unittest or scripts within.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 29, 2017

@tommikas Can you please review the history of jenkins CI , are there any problems today we should be aware?

Arto Kinnunen added 3 commits November 29, 2017 21:34
…changes from 0697d9a..c9bf20f

c9bf20f Remove test and unittest folders

git-subtree-dir: features/nanostack/FEATURE_NANOSTACK/sal-stack-nanostack
git-subtree-split: c9bf20f
…k_release_v704

* commit '041b6fa73681061072f970e9cf11ff4d422fa04b':
  Squashed 'features/nanostack/FEATURE_NANOSTACK/sal-stack-nanostack/' changes from 0697d9a..c9bf20f
@sg-
Copy link
Contributor

sg- commented Nov 30, 2017

Why is Nanostack still in a FEATURE_ directory? I'd expected this would always be part of the build now...

So feature/nanostack/ Soon all FEATURE_ will be gone and we can move these to the root level like /networking/

@artokin
Copy link
Contributor Author

artokin commented Nov 30, 2017

@sg- , we still like to use existing FEATURE-configuration to include/exclude Nanostack from the application build. This ensures that existing applications are still working without surprises.

@sg-
Copy link
Contributor

sg- commented Nov 30, 2017

When will that change and be part of the default build?

@TuomoHautamaki
Copy link

@sg- I'll contact you to discuss more about this future change. Not in the scope of this release.

@artokin
Copy link
Contributor Author

artokin commented Nov 30, 2017

Unit tests are now removed from Nanostack. All items should be solved now (unit tests in other components will be addressed separately).

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 30, 2017

/morph build

@mbed-ci
Copy link

mbed-ci commented Nov 30, 2017

Build : SUCCESS

Build number : 625
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5602/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

@mbed-ci
Copy link

mbed-ci commented Nov 30, 2017

@mbed-ci
Copy link

mbed-ci commented Nov 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants