Skip to content

Filesystem Class added to mbed namespace #7747

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

Closed
wants to merge 1 commit into from

Conversation

deepikabhavnani
Copy link

@deepikabhavnani deepikabhavnani commented Aug 9, 2018

Description

Moving FatFs and LittleFs inside mbed namespace. More info : #7663

Please note:
All disk operations implemented for ChanFs (inside FATFileSystem.cpp file) are excluded from mbed namespace. https://github.com/ARMmbed/mbed-os/blob/master/features/filesystem/fat/FATFileSystem.cpp#L140

Pull request type

[] Fix
[ ] Refactor
[ ] Target update
[ ] Feature
[X] Breaking change

Targeted to minor release

Copy link
Contributor

@geky geky left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@@ -205,8 +206,8 @@ DRESULT disk_read(BYTE pdrv, BYTE *buff, DWORD sector, UINT count)
{
Copy link
Contributor

@geky geky Aug 9, 2018

Choose a reason for hiding this comment

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

Could the disk functions still live in the mbed namespace if you declare them as extern "C"?

Copy link
Member

Choose a reason for hiding this comment

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

If they are declared as extern "C" then they are part of the global namespace; the only one C knows.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though declaring them as extern "C" allows you to use anything from the mbed namespace and also notates why the functions aren't in the namespace.

It's just a minor thing to note

@pan-
Copy link
Member

pan- commented Aug 10, 2018

Why is the breaking change box not ticked ?

@geky
Copy link
Contributor

geky commented Aug 10, 2018

FYI should probably wait for resolution of #7757

@deepikabhavnani
Copy link
Author

Need to wait for major version release.

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.

5 participants