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+mbed-client libservice update #4715

Merged
merged 2 commits into from Jul 27, 2017

Conversation

Projects
None yet
7 participants
@kjbracey-arm
Contributor

kjbracey-arm commented Jul 6, 2017

Update Nanostack/mbed-client libservice subtree - to provide new nsdynmemLIB API added in ARMmbed/nanostack-libservice#60

Changes are an API extension, so should be no backwards compatibility issues.

Update required for mbed-client - discussed with @yogpan01

Squashed patch contains only that nsdynmemLIB change, plus the Yotta change in ARMmbed/nanostack-libservice#59, which does not affect mbed-os.

kjbracey-arm added some commits Jul 6, 2017

Squashed 'features/FEATURE_COMMON_PAL/nanostack-libservice/' changes …
…from 6444c8a..854505d

854505d Introduce object oriented dynmemLIB
27bae6b Merge pull request #59 from ARMmbed/fix_yotta_deps
ff45891 Add dependency on mbed-trace

git-subtree-dir: features/FEATURE_COMMON_PAL/nanostack-libservice
git-subtree-split: 854505d0191e428c07eb25fc9630dc05b21d5a47
Merge commit '0613e29fde32daa609eb6ae4ad785112b8fd9729'
* commit '0613e29fde32daa609eb6ae4ad785112b8fd9729':
  Squashed 'features/FEATURE_COMMON_PAL/nanostack-libservice/' changes from 6444c8a..854505d
@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Jul 6, 2017

@adbridge

This comment has been minimized.

Contributor

adbridge commented Jul 6, 2017

@kjbracey-arm Hi Kevin if "Merge commit '0613e29fde32daa609eb6ae4ad785112b8fd9729'" , if this is actually a merge commit could you please replace with a rebase as merge commits cause problems with our automated release tools. Thanks

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Jul 6, 2017

It is a merge commit - a necessary artifact of the git subtree maintenance system already in use for this directory.

The last few FEATURE_COMMON_PAL and lwIP core updates went through as similar git subtree merges, so I thought it was semi-routine by now. (The subtrees are working rather smoothly at our end at least - updates are super-easy to do and track).

Many other pull requests have embedded merges in (from master), and I can't think why this merge commit should cause any worse problems than that. If it's because it's a merge right at the tip of the PR branch, would adding a dummy empty commit at the tip help workaround the issue?

@adbridge

This comment has been minimized.

Contributor

adbridge commented Jul 7, 2017

@kjbracey-arm The majority of the merge commits against master cause problems when they bring in all the commits from the merged branch. Our release script creates a patch from every commit on a PR and then applies to the release branch using git am, so this causes issue when a list of unrelated commits also get dragged in. Usually as a consequence of someone not keeping their local branches up to date and having to resolve conflicts . As it goes this one looks quite clean and may not cause us issues.

@kjbracey-arm

This comment has been minimized.

Contributor

kjbracey-arm commented Jul 7, 2017

Thanks for the info. There's no conflict in the merge, so git am on the other "content" squash commit should work fine.

(These subtrees would not normally produce merge conflicts, as the target folder generally isn't being otherwise touched in this repository - it's just a copy of the master repo).

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 10, 2017

@SeppoTakalo, @adbridge, @0xc0170 Could you review?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 17, 2017

/morph test-nightly

@yogpan01

This comment has been minimized.

Contributor

yogpan01 commented Jul 17, 2017

@0xc0170 How about this PR ? When can this be merged and which release can we get it ?

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 17, 2017

@0xc0170 How about this PR ? When can this be merged and which release can we get it ?

Waiting for CI, possibly the next one as previous PR

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 17, 2017

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 821

Example Build failed!

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 24, 2017

Restarting, Failure does not seem to be realted to this changeset, rather a script failure

/morph test-nightly

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 24, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 867

All builds and test passed!

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Jul 26, 2017

@theotherjimmy theotherjimmy merged commit a0c9c88 into ARMmbed:master Jul 27, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test-nightly Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment