#include "../../something/something.hpp" is not easy to read #6
Conversation
There was a problem hiding this comment.
Before accepting this change, I must make sure it still work with ESP32's build system (that's were I'm using it currently). It's a little bit tricky to use their build system and include path is a pain to get right. I'll try the build when I'm home and let you know if it's ok.
| @@ -2,10 +2,10 @@ | |||
| #define hpp_CPP_MQTTClient_CPP_hpp | |||
|
|
|||
| // Configuration is done via macros | |||
| #include "MQTTConfig.hpp" | |||
| #include <Network/Clients/MQTTConfig.hpp> | |||
There was a problem hiding this comment.
This is wrong, IMHO. A file in the same folder must stay with its "" include path.
There was a problem hiding this comment.
I think there is such a way of thinking. However, since this is a library, it is kinder to write the include path specification from the file in the user-visible path to the user-visible file in a general format (<something/something.hpp>) so that it can be used as a reference when the user includes it by the user.
Ah, that certainly has to be confirmed. I can't cooperate because I don't have the ESP32 right now. I'm sorry. |
We can delete warning message for using "#pragma message" is not recommended by set -Wno-#pragma-messages to clang. but we can't delete warning completely using -Wno-#pragma-messages because contents of message is also treated as warning.
|
You have removed a lot of configuration code from the project, so I can't accept this patch as is. I've implemented and reincluded the ideas you've spotted here, it's pushed in master. I advise you to merge the change to ensure it fits your need. Cheers! |
|
Please close this PR without merge. |
|
Ok, so I'm closing this. Have a nice day! |
close #5
#include <something/something.hpp> is better. There is no problem if this PR is not merged, but since I noticed this, I issued it as PR.