MAJOR CHANGE (but not really): Move headers to src/ directory #504
Conversation
- Move all /include/**/*.h to /src/... (with same hierarchy) - Move /src/examples to /examples - Move /src/Qt/demo to /examples/qt-demo - Adjust source files (far fewer relative includes): - /src/**/*.cpp - /src/**/*.h - examples/**/*.cpp - tests/**/*.cpp - Eliminate relative includes from all examples/, tests/ source files - Adjust all CMakeLists.txt files to account for new include path - Add an examples/CMakeLists.txt, and move the logic for compiling and installing the necessary components to that file - Add logic to tests/CMakeLists, examples/CMakeLists, /CMakeLists.txt so that build components are imported locally when needed - Update Doxygen.in file to build docs from new locations
|
Oh, I forgot to mention: This shouldn't affect anyone's builds or use of the library, since the installed output is exactly the same. It just takes a different path getting there. But it will affect developers, obviously, including anyone running OpenShot out of a build dir with |
- Makes the generated documentation cleaner
- Use a generated 120x120px openshot-qt.png as logo image - Add summary from project metadata to page header - Turn off STL support, was cluttering diagrams with std::string refs - Fix some errant documentation comments in the source files
Codecov Report
@@ Coverage Diff @@
## develop #504 +/- ##
========================================
Coverage 48.24% 48.25%
========================================
Files 128 128
Lines 9962 9960 -2
========================================
Hits 4806 4806
+ Misses 5156 5154 -2
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## develop #504 +/- ##
========================================
Coverage 48.33% 48.34%
========================================
Files 128 128
Lines 9952 9950 -2
========================================
Hits 4810 4810
+ Misses 5142 5140 -2
Continue to review full report at Codecov.
|
|
@ferdnyc This one seems scary to me... and lots of conflicts. I might wait on this for now. I think this is more of an emotional response, vs a technical one. Technically, nothing is wrong with this at all. |
Honestly, the only way to do something like this is to set it up and then commit it immediately, or pretty close. It touches every header, so conflicts are inevitable and they pile up fast... but it's mostly just a few quick If you're down with the concept, I'll re-create this as a new PR and we can get it merged quick before the conflicts pile up. |
This implements the proposal I made in #471, that header files live in the
src/directory alongside their*.cppfiles, rather than being kept in aninclude/directory./include/**/*.hto/src/...(with same hierarchy)/src/examplesto/examples/src/Qt/demoto/examples/qt-demo/src/bindings/to/bindings/src/**/*.cpp/src/**/*.hexamples/**/*.cpptests/**/*.cpp#include "JuceHeader.h"to#include <JuceHeader.h>everywhere, since that header isn't part of our local source for the purposes of a libopenshot buildCMakeLists.txtfiles to account for new include pathexamples/CMakeLists.txt, and move the logic for compiling and installing the necessary components to that filetests/CMakeLists,examples/CMakeLists,/CMakeLists.txtso that build components are imported locally when neededDoxygen.infile to build docs from new locationsThis will be confusing, at first, when looking for moved files. (I've confused myself a couple of times already.) The relocated
bindingsdirectory is the one that throws me the most. But it really is much simpler this way. Both in terms of humans editing the code, and in terms of the contents of the code.The reason
src/examples,src/Qt/demo, andsrc/bindingsgot moved out ofsrc/is in small part practical: If there are directories insrc/without any installable headers, theinstall(DIRECTORY...)CMake command will end up creating empty directories in the output location. So, I wanted to avoid that.But at the same time, the fact that that happens makes the point: none of those directories' contents are part of the libopenshot source code itself. So they really don't belong in the libopenshot source dir. It makes more sense if they're separated.
The Doxygen docs mostly come out better, now. It never could follow the parent-relative
#includes, so in the past none of the source code listings'#includes linked to the header in question. Now almost all of them do, except for a few#include "../foo.h"statements in thesrc/effects/orsrc/Qtpaths. (And in theory I could eliminate those using the include path, since#include "ReaderBase.h"should mean the same thing insrc/effects/Blur.hthat it means anywhere else. But I haven't tackled that for this PR, so it [still] uses#include "../ReaderBase.h".)I've tested this in CMake 3.2 and CMake 3.17 and everything worked, but I've only tested it on Linux. This branch is in the project repo, so the builders can chew on it in addition to Travis. (And if we merge #501, also AppVeyor.)