Fix #2750 #2763
Fix #2750 #2763
Conversation
libraries/SPIFFS/src/SPIFFS.cpp
Outdated
| { | ||
| } | ||
|
|
||
| bool SPIFFSImpl::exists(const char* path) { |
There was a problem hiding this comment.
please use a new line before the curly braces. I see this a few more times below :)
libraries/SPIFFS/src/SPIFFS.cpp
Outdated
| @@ -1,32 +1,26 @@ | |||
| // Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD | |||
libraries/SPIFFS/src/SPIFFS.h
Outdated
| @@ -1,39 +1,33 @@ | |||
| // Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD | |||
libraries/SPIFFS/src/SPIFFS.h
Outdated
| class SPIFFSImpl : public VFSImpl | ||
| { | ||
| public: | ||
| SPIFFSImpl(); |
|
Left a few styling notes. Not sure why you deleted the company headers? |
|
|
I merged all of the recent changes into my fork before committing fixes for this. Not sure what effect that will have on attempts by you to merge this specific change to SPIFFS. |
libraries/SPIFFS/src/SPIFFS.h
Outdated
| #include "FS.h" | ||
| #include "FSImpl.h" |
There was a problem hiding this comment.
these two includes should not be in the header, but in the CPP file instead. Nothing in the header depends on them :)
There was a problem hiding this comment.
The header does depend on them, but they are also included via vfs_api.h, so at most they are redundant. For the same reason they don't need to be included in the .cpp file (because they are included via vfs_api.h which is included via SPIFFS.h).
Whatever, I have removed them from SPIFFS.h, but I haven't added them to SPIFFS.cpp.
libraries/SPIFFS/src/SPIFFS.h
Outdated
| @@ -14,26 +14,37 @@ | |||
| #ifndef _SPIFFS_H_ | |||
| #define _SPIFFS_H_ | |||
|
|
|||
| #pragma once | |||
There was a problem hiding this comment.
this is redundant with the above. keep one or the other :)
libraries/SPIFFS/src/SPIFFS.h
Outdated
| @@ -14,26 +14,33 @@ | |||
| #ifndef _SPIFFS_H_ | |||
| #define _SPIFFS_H_ | |||
|
|
|||
| #include "FS.h" | |||
| #include "vfs_api.h" | |||
There was a problem hiding this comment.
FS should be included. vfs_api is a private header.
There was a problem hiding this comment.
18 hours ago, you asked me to remove FS.h and FSImpl.h, now you are asking me to add FS.h back in.
If I remove vfs_api.h I will also have to add FSImpl.h back because SPIFFSImpl is declared in SPIFFS.h.
There was a problem hiding this comment.
No, I asked about #include "FSImpl.h" and #include "vfs_api.h".
FS.h is the only one that should be included :) I guess a bit of misunderstanding.
There was a problem hiding this comment.
This change requires an additional class, SPIFFFSImpl that inherits VFSImpl, which is declared in vfs_api.h, so something is going to need to include vfs_api.h. Currently this class is declared in SPIFFS.h, which is why SPIFFS.h includes vfs_api.h. How would you have me resolve this?
There was a problem hiding this comment.
Does this class need to be public? What happens if the declaration goes into the CPP file?
There was a problem hiding this comment.
It should be fine if I move it to the CPP file.
|
merged :) |
This is a fix for #2750