Some ideas on software infrastructure improvement

[faisal-bhuiyan]

I am proposing the following enhancements to the OpenTurbine SW infrastructure:

• Create API documentation via Doxygen - Use of doxygen specific annotation embedded within C++ source code is pretty common to generate API documentation with minimal effort, e.g. in AMR wind.

• Adopt a version number for project in CMakeLists.txt - we might consider adopting a version number for OpenTurbine using CMake. We could use the version number along with a log file (discussed below) to quickly check the build version for development as well as for releases.

• Create a logging utility - a logging utility for recording log messages to files (alongside the console) should be a very useful development tool and the first point of debugging whenever necessary. Various open-source libraries are available for this such as spdlog, however, we could just implement our own basic logging utility with minimal effort.

• Adopt a C++ Style guide - We are currently using the C++ style guide in the form of a clang-format as recommended in amr-wind. However, having a descriptive style guide helps to know about the thought process behind the choices made. The Google C++ style guide might be a good candidate to this end, of course with a few modifications of our own, e.g., increasing the line length from 80 characters to a more reasonable value such as 120.

I appreciate your valuable feedback on the above items and looking forward to a productive discussion. Thanks!

[alanw0]

@faisal-bhuiyan I definitely agree with adopting a version number, and a C++ style guide.

For Doxygen documentation, I would advocate only using that for public APIs, not for every function/class in the project. It can be useful for users, but not as much for internal developers. For developers, the best documentation regarding how anything works, is the unit-tests. Also, you always know that unit-tests are up-to-date if they are building and passing. Doxygen documentation often falls out of date.

I agree that logging can be useful, although it also has the risk of falling out of date. developers need to be diligent about logging things. Also, generally it needs to be turned on or off at compile time in order to not hurt performance.

[faisal-bhuiyan]

For Doxygen documentation, I would advocate only using that for public APIs, not for every function/class in the project. It can be useful for users, but not as much for internal developers. For developers, the best documentation regarding how anything works, is the unit-tests. Also, you always know that unit-tests are up-to-date if they are building and passing. Doxygen documentation often falls out of date.

I agree that Doxygen might not be very useful for o-turb developers, however it should have some value for users of the codebase to learn about the API. I do agree this has the potential to become outdated, however given the very little additional effort required to add comments to header files (which might have been warranted anyways), I think it it worth it.

I agree that logging can be useful, although it also has the risk of falling out of date. developers need to be diligent about logging things. Also, generally it needs to be turned on or off at compile time in order to not hurt performance.

I agree with both points. We should be able to set the proper logging level at compile time to not hurt performance and print unnecessary values that might be required for debugging only. I believe logging would add enough value to a project to motivate devs to keep the logging functionality up-to-date, however, I would take your advice if you think it's not worth our efforts.

[rafmudaf]

@faisal-bhuiyan In response to the versioning, it's worth noting the current system. The main CMakeList calls the cmake macro generate_git_info in openturbine-utils.cmake. That macro uses a few routines in GetGitRevisionDescription.cmake to parse the git-status. Ultimately, OpenTurbineVersion.H is generated based on the results from OpenTurbineVersion.H.in in the src/CMakeLists.txt. This header is currently only used to display the version info in the CLI and unit test driver:

$ ./openturbine -h
==============================================================================
                OpenTurbine (https://github.com/exawind/openturbine)

  OpenTurbine version :: v0.0.0-37-gcef9bff-DIRTY
  OpenTurbine Git SHA :: cef9bffc11001faba73520580ba129bc5d3a8ef7-DIRTY
  Exec. time       :: Thu Feb  9 13:56:51 2023

  No additional third-party libraries enabled

           This software is released under the MIT License.                
 See https://github.com/Exawind/openturbine/blob/main/LICENSE for details. 
------------------------------------------------------------------------------

Note this is copied from a comment in #9 since its more appropriate to discuss the design of this system.

[faisal-bhuiyan]

Thanks for taking the time to explain the current versioning system, this makes sense to me. We should be able to leverage this functionality whenever we need to report the build version within source code and not maintain another version information within CMake.

[faisal-bhuiyan]

In addition to the above, I propose the usage of #pragma once preprocessor directive in favor of include guards. So the following

#ifndef _MYHEADER_H_
#define _MYHEADER_H_
...
#endif

will be replaced by simply adding:
#pragma once

This appears to be the common practice and more efficient to achieve the same result, i.e. compile a header file only once. Let me know if I am missing something here.

[alanw0]

Yes I would agree with this. #pragma once is not standard, but it is common practice and supported by pretty much every compiler/preprocessor so I think it's probably safe.