Remove FEATURE_STORAGE and all underlying deprecated features #10258

davidsaada · 2019-03-28T16:46:08Z

Description

FEATURE_STORAGE and underlying features - cfstore, flash-journal and storage-volume-manager, have long been deprecated. This PR removes them all.
In addition, remove references to these in nanostack (used cfstore in uninvoked tests) and tools metadata such as astyle and doxygen.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@SenRamakri

Release Notes

Remove FEATURE_STORAGE and all underlying deprecated features: cfstore, flash-journal and storage-volume-manager.

ciarmcom · 2019-03-28T18:00:37Z

@davidsaada, thank you for your changes.
@SenRamakri @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

theotherjimmy · 2019-03-28T18:55:47Z

tools/test/config/feature_recursive_add/test_data.json

@@ -1,6 +1,6 @@
 {
    "test_target": {
-        "expected_features": ["BOOTLOADER", "STORAGE"],
-        "included_source": ["FEATURE_BOOTLOADER/lib1/lib1.c", "FEATURE_STORAGE/lib2/lib2.c"]
+        "expected_features": ["BOOTLOADER"],


Please don't remove code that tests how the tools work WRT features just because the feature that's used as test data is no longer part of Mbed OS

I don't mind reverting this file, but will this work even if STORAGE feature and FEATURE_STORAGE directory no longer exist?
Alternatively, I can replace these with other feature and directory, if this only requires any two features.

Yes. it uses a local folder. You're welcome to rework it to not use the string "STORAGE" if you're so inclined, but I don't think it's worth the time.

Thanks. Indeed got better things to do with my time. Reverted.

SenRamakri · 2019-03-28T20:33:32Z

@cmonr and @ARMmbed/mbed-os-maintainers - Can we please tag this change for 5.12.1. Without this change we cannot compile with ARM Compiler 6.12 which is the ARM compiler version with latest Keil MDK. Im afraid there will be lot of developers trying to use latest Keil MDK with MbedOS and the builds will be broken. So we need this change in ASAP(5.12.1).

cmonr · 2019-03-28T20:55:10Z

@SenRamakri @davidsaada

I'm not sure I understand. Why does removing this code happen to make compilation work with ARMC6?

And where is the tracking issue for this removal? Was it discovered during OOB?

davidsaada · 2019-03-28T21:02:44Z

@cmonr We had this change planned anyway. As for the ARMC6 problem, will let @SenRamakri answer that.

SenRamakri · 2019-03-28T21:39:31Z

@cmonr - Its not discovered during OOB because OOB used ARMC 6.11 version. We have identified some header file conflicts with ARMC 6.12 with header files present under the directories which @davidsaada is trying to remove with this commit. I brought this up to him yesterday and he prioritized this change to fix ARMC 6.12 issue although it was planned for later.

SenRamakri

@davidsaada - Changes look good to me, Ill approve.
Btw, are there any docs which also needs removed or changed based on this change?

adbridge · 2019-03-29T13:19:57Z

My concern with this PR is that we explicit state in our guidelines that we do not deprecate features in patch releases. If there is a specific fix that is required for ARMC6 then I would prefer to see that split out into a separate PR that we can take into a patch release and then move the deprecation to the next minor...

bulislaw · 2019-03-29T13:23:28Z

Mbed deprecation policy says that we can remove features that were marked as deprecated for at least 6 months. Can you confirm all of the removed items were explicitly marked as deprecated for at least 6 months?

Also that will need to go into 5.13 not any of the 5.12 patch releases.

davidsaada · 2019-03-29T13:47:42Z

@adbridge @SenRamakri @bulislaw All three features have been deprecated since mbed-os 5.5.
Neither have any reference in our documentation.

SenRamakri · 2019-03-29T13:49:25Z

Sounds good - But we need "VERSION" file removed to fix the ARMC 6.12 build failures for 5.12.1.
@davidsaada - Is that ok to just remove the VERSION file for now with a separate PR and target this PR for 5.13?

davidsaada · 2019-03-29T13:53:03Z

Sounds good - But we need "VERSION" file removed to fix the ARMC 6.12 build failures for 5.12.1.
@davidsaada - Is that ok to just remove the VERSION file for now with a separate PR and target this PR for 5.13?

Will do (on Sunday). In that case I believe a tracking issue would help.

0xc0170 · 2019-04-11T12:04:33Z

@davidsaada What's the status here? The fix was merged, can this proceed?

davidsaada · 2019-04-11T12:06:46Z

@davidsaada What's the status here? The fix was merged, can this proceed?

From my POV it can proceed. Nothing more I have to do here. But thought this would wait for 5.13.

Review addressed

0xc0170 · 2019-04-11T13:05:04Z

CI started while waiting for reviews

mbed-ci · 2019-04-11T16:10:04Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_exporter

cmonr · 2019-04-12T01:39:08Z

CI job restarted: jenkins-ci/exporter

Failures appear relevant, but making sure.

0xc0170 · 2019-04-12T08:50:56Z

Restarted, latest failure was success but exporter reported failure

0xc0170 · 2019-04-12T13:22:37Z

@bulislaw Please review

mbed-ci · 2019-04-17T02:01:37Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

bulislaw

Could you like to PR with docs changes?

adbridge · 2019-04-25T15:20:17Z

ci started

mbed-ci · 2019-04-25T22:11:04Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 3
Build artifacts

ciarmcom requested review from SenRamakri and a team March 28, 2019 18:00

ciarmcom added the needs: review label Mar 28, 2019

cmonr added the release-version: 5.13.0-rc1 label Mar 28, 2019

cmonr requested a review from bulislaw March 28, 2019 18:20

theotherjimmy previously requested changes Mar 28, 2019

View reviewed changes

davidsaada force-pushed the david_remove_feature_storage branch from 26f7b06 to 2a771fc Compare March 28, 2019 20:56

Remove FEATURE_STORAGE and all underlying deprecated features

732dd36

davidsaada force-pushed the david_remove_feature_storage branch from 2a771fc to 732dd36 Compare March 28, 2019 21:11

SenRamakri approved these changes Mar 29, 2019

View reviewed changes

davidsaada mentioned this pull request Mar 31, 2019

Remove CFSTORE VERSION file (colliding with an ARM 6.12 C++ header file) #10280

Merged

0xc0170 approved these changes Apr 11, 2019

View reviewed changes

0xc0170 removed the request for review from a team April 11, 2019 13:02

0xc0170 added needs: CI and removed needs: review labels Apr 11, 2019

0xc0170 added needs: review and removed needs: CI labels Apr 12, 2019

bulislaw approved these changes Apr 17, 2019

View reviewed changes

adbridge added needs: CI and removed needs: review labels Apr 25, 2019

adbridge added ready for merge and removed needs: CI labels Apr 26, 2019

adbridge merged commit b1cd3da into ARMmbed:master Apr 26, 2019

adbridge removed the ready for merge label Apr 26, 2019

Remove FEATURE_STORAGE and all underlying deprecated features #10258

Remove FEATURE_STORAGE and all underlying deprecated features #10258

Uh oh!

Conversation

davidsaada commented Mar 28, 2019

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Mar 28, 2019

Uh oh!

theotherjimmy Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

davidsaada Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

theotherjimmy Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

davidsaada Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

SenRamakri commented Mar 28, 2019

Uh oh!

cmonr commented Mar 28, 2019

Uh oh!

davidsaada commented Mar 28, 2019

Uh oh!

SenRamakri commented Mar 28, 2019

Uh oh!

SenRamakri left a comment

Choose a reason for hiding this comment

Uh oh!

adbridge commented Mar 29, 2019

Uh oh!

bulislaw commented Mar 29, 2019

Uh oh!

davidsaada commented Mar 29, 2019

Uh oh!

SenRamakri commented Mar 29, 2019

Uh oh!

davidsaada commented Mar 29, 2019

Uh oh!

0xc0170 commented Apr 11, 2019

Uh oh!

davidsaada commented Apr 11, 2019

Uh oh!

0xc0170 commented Apr 11, 2019

Uh oh!

mbed-ci commented Apr 11, 2019

Test run: FAILED

Uh oh!

cmonr commented Apr 12, 2019

Uh oh!

0xc0170 commented Apr 12, 2019

Uh oh!

0xc0170 commented Apr 12, 2019

Uh oh!

mbed-ci commented Apr 17, 2019

Test run: FAILED

Uh oh!

bulislaw left a comment

Choose a reason for hiding this comment

Uh oh!

adbridge commented Apr 25, 2019

Uh oh!

mbed-ci commented Apr 25, 2019

Test run: SUCCESS

Uh oh!

Uh oh!