kImageAnnotator icon indicating copy to clipboard operation
kImageAnnotator copied to clipboard

Development issues

Open fpohtmeh-github opened this issue 3 years ago • 3 comments

I worked on my first issue here and collected few things that make development uncomfortable.

  1. Active usage of includes in headers. Please replace with forward declarations where it's possible. Otherwise developer gets long-term rebuilds after a single change in header.
  2. Inconsistent or mixed tab/space policy.
  3. (Minor) src/CMakeLists.txt includes source files only. Please include corresponding header files as well. This will improve project navigation and show header-only classes in project tree.

P.S. I can work on it after zooming.

fpohtmeh-github avatar Aug 04 '20 17:08 fpohtmeh-github

Thanks for bringing this to my attention.

  1. I didn't think that this would be a problem as our classes are rather small. On the other hand, it's not like have thought about it too much. If you think that there is a noticeable compile time improvement, we should probably go for it.
  2. Sorry for that, might come from me using different machines for developing (Windows, Mac, Linux, different VMs) and not fully setting up my IDE. Tab should be used everywhere.
  3. I'm not sure about this point as it would be additional work to add the header files, also additional work with renaming files as you would have to rename always two files and makes the CMakeList.txt larger and harder to navigate. Most of the CMake based projects that I have seen so far include only the source files or that's at least what I remember seeing. What IDE are you using? Is it possible to switch to a directory based one?

DamirPorobic avatar Aug 10 '20 10:08 DamirPorobic

looking at your PR, I see that you put your includes into the implementation file. Is this related to 1.?

DamirPorobic avatar Sep 07 '20 15:09 DamirPorobic

looking at your PR, I see that you put your includes into the implementation file. Is this related to 1.? Yes, just a good practice to minimize build time

fpohtmeh-github avatar Sep 10 '20 16:09 fpohtmeh-github