search

ARMmbed / core-util

DEPRECATED: Mbed 3 utilities library
Other
12 stars 17 forks source link

rename subfolder mbed-util to core-util #27

Closed rgrover closed 8 years ago

rgrover commented 8 years ago

@0xc0170 @bogdanm @bremoran

rgrover commented 8 years ago

tested against mbed-drivers using frdm-k64f-gcc. All previously passing tests continue to pass.

0xc0170 commented 8 years ago

Is this just rename? Github shows some changes in files within the same commit :confused: Can we split them into separate PR?

rgrover commented 8 years ago

sorry. it seems some changes from my work for the POSIX backend have entered into these PRs. I'll filter them out. The renames made it difficult to compare.

0xc0170 commented 8 years ago

what about namespace rename (mbed::util -> core::util) ?

rgrover commented 8 years ago

I'm happy to rename namespace mbed::util to core::util. Please object if I shouldn't.

yogpan01 commented 8 years ago

renaming namespace will cause constant changes to modules which are using for example mbed-client-example will break , also mbed-mesh-api will break. Please re-consider these before you go ahead and change namespacing

bogdanm commented 8 years ago

what about namespace rename (mbed::util -> core::util) ?

I was in vacation for too long it seems :) When did this happen?

rgrover commented 8 years ago

@bogdanm mbed::util -> core::util hasn't happened yet. Based on @yogpan01's comment, might not happen just yet.

bogdanm commented 8 years ago

@bogdanm mbed::util -> core::util hasn't happened yet. will happen shortly. (today?)

Sorry @rgrover, I wasn't clear. My question was "why do we want to change this?"

rgrover commented 8 years ago

@bogdanm it would seem a logical fallout of renaming everything as core-util. But perhaps it isn't necessary.

0xc0170 commented 8 years ago

@bogdanm I am asking if all this rename should also impact namespace in this module to follow, or we are happy with mbed::util for future

@yogpan01 This PR is breaking change already (we should create a label in our repositories to indicate the intention (breaking change) (does not break modules only if they use the feature "extraInclude" (not mbed-util/header_util.h, but header_util.h)

bremoran commented 8 years ago

@0xc0170 @yogpan01: This is what sem-ver is for. This will be a minor version bump. All new development will continue in the new minor version. All modules depending on core-util will need to be updated to build cleanly before adopting the new version.

rgrover commented 8 years ago

please review @0xc0170 @bogdanm @bremoran

bremoran commented 8 years ago

We should see if we can get Jenkins to cover this repo, given all the changes that are active.

bogdanm commented 8 years ago

+1 @bremoran

0xc0170 commented 8 years ago

@rgrover Can you please update also https://github.com/rgrover/core-util/blob/master/module.json ? I noticed there is still old name used.

rgrover commented 8 years ago

@0xc0170 please review

rgrover commented 8 years ago

@bogdanm please review

rgrover commented 8 years ago

@bogdanm

bremoran commented 8 years ago

The loss of Jenkins earlier today meant that we haven't gotten the build triggers for these changes. I've manually triggered a build.

The build failed on all platforms with a missing header file. https://jenkins-internal.mbed.com/job/core-util/3/

I suspect that these are dangling references to repos with PRs already open for this change. I see one failure in minar and one in ualloc

bogdanm commented 8 years ago

LGTM. The reported build failure should be fixed by a separate PR to MINAR, so I'm going to assume that you compiled locally and everything was fine.

rgrover commented 8 years ago

I've compiled locally and run tests from mbed-drivers

bogdanm commented 8 years ago

+1 then

bremoran commented 8 years ago

+1

bogdanm commented 8 years ago

Published new minor version 0.2.0