InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.4k stars 662 forks source link

STYLE: Remove doxygen.config.in (superseded by DoxygenConfig.cmake) #4500

Closed N-Dekker closed 5 months ago

N-Dekker commented 5 months ago

The use of doxygen.config has been replaced by DoxygenConfig.cmake, with pull request https://github.com/InsightSoftwareConsortium/ITK/pull/3311 commit a96d03fb5f4a4bb470516aa623dbc9b4aaaa6e4f "COMP: Use modern doxygen_add_docs for BUILD_DOCUMENTAITON", Hans Johnson (@hjmjohnson), March 18, 2022

thewtex commented 5 months ago

@N-Dekker did you confirm that the doxygen build succeeds?

N-Dekker commented 5 months ago

did you confirm that the doxygen build succeeds?

@thewtex I did not try it out locally, but I searched exhaustively for "doxygen.config" in the source tree, and only found it in comments, like:

 # The DoxygenConfig.cmake is a replacement for configuring a doxygen.config.in file

https://github.com/InsightSoftwareConsortium/ITK/blob/e3189492ce6858035b68a2b89e4a4d2f1d4041ad/Utilities/Doxygen/CMakeLists.txt#L171

Before commit a96d03fb5f4a4bb470516aa623dbc9b4aaaa6e4f (March, 2022), Utilities/Doxygen/CMakeLists.txt still configured doxygen.config.in, by doing:

configure_file(${ITK_SOURCE_DIR}/Utilities/Doxygen/doxygen.config.in
               ${ITK_BINARY_DIR}/Utilities/Doxygen/doxygen.config)

That statement was removed by commit a96d03fb5f4a4bb470516aa623dbc9b4aaaa6e4f (March, 2022).

@hjmjohnson Can you confirm that "doxygen.config.in" is no longer necessary, now that we have DoxygenConfig.cmake?

N-Dekker commented 5 months ago

This pull request may also be considered as a proof that "doxygen.config.in" is no longer used:

Apparently it was sufficient to add itkVirtualGetNameOfClassMacro and itkOverrideGetNameOfClassMacro to "DoxygenConfig.cmake", in order to get GetNameOfClass() documented at https://itk.org/Doxygen/html/ It is no longer necessary to add such macro's to "doxygen.config.in", because "doxygen.config.in" is ignored anyway, nowadays.

thewtex commented 5 months ago

@N-Dekker it needs to be tested before merging. I will start a local build.