You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2020/12/30 18:29:12 UTC
[GitHub] [logging-log4cxx] rm5248 opened a new pull request #49: Documentation updates
rm5248 opened a new pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49
This PR does the following:
* Convert apt/xdoc documentation to markdown
* Use doxygen for site generation instead of maven
* Let CMake generate the tar.gz/zip files for releases, as well as signing them
* Recreate the important generated pages that maven generates in markdown(download, mailing list, source repository)
* Added a new documentation page with some configuration samples
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening edited a comment on pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
ams-tschoening edited a comment on pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#issuecomment-757520874
How to enable generating a TAR etc. for releases:
How to enable generating the SITE:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r554977381
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
+include(CPack)
+
+add_custom_target( dist
+ COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} -- package_source
+ COMMAND ${CMAKE_COMMAND} -E sha512sum "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}.tar.gz" > "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}.tar.gz.sha512"
Review comment:
I was looking in the wrong directory, `_CPack_Packages` only. But the archives from there get copied into `out\build\x64-Debug` in the end and the SHA-files were properly there.
No need to to make things additionally complicated for `gpg` right now. In the worst case, missing signatures will most likely be spotted anyway.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r554610161
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
+include(CPack)
+
+add_custom_target( dist
Review comment:
How do you execute that target, on the shell and with which command? Visual Studio doesn't seem to provide anything obvious in its UI, `installing` doesn't seem to trigger that target. Adding `ALL` like for Doxygen does, but that's not what you intended?
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
Review comment:
I have more things to exclude, some of those even preventing things from working:
```
Schweregrad Code Beschreibung Projekt Datei Zeile Unterdrückungszustand
Fehler Problem copying file: C:/Users/tschoening/Documents/Svn/Src/Libs/trunk/C++/Logging/log4cxx/master/src/.vs/src/v16/Browse.VC.db -> C:/Users/tschoening/Documents/Svn/Src/Libs/trunk/C++/Logging/log4cxx/master/src/out/build/x64-Debug/_CPack_Packages/win64-Source/TGZ/apache-log4cxx-0.11.0////.vs/src/v16/Browse.VC.db C:\Users\tschoening\Documents\Svn\Src\Libs\trunk\C++\Logging\log4cxx\master\src\out\build\x64-Debug\src C:\Users\tschoening\Documents\Svn\Src\Libs\trunk\C++\Logging\log4cxx\master\src\out\build\x64-Debug\EXEC 1
```
I need the following instead:
> set(CPACK_SOURCE_IGNORE_FILES ".git/;.vs/;build/;out/")
Your `build` is `out/build` and `out/install` for me. That's configured in my JSON file, though I'm not sure if that was a default of Visual Studio or a decision I made.
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
+include(CPack)
+
+add_custom_target( dist
+ COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} -- package_source
+ COMMAND ${CMAKE_COMMAND} -E sha512sum "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}.tar.gz" > "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}.tar.gz.sha512"
Review comment:
`sha512sum` is simply a binary expected to be available using `PATH`, correct? Because in my case it isn't, but the build seems to succeed. I didn't see any error message that this command or that for `gpg` failed. Though, only the archives themself got created.
Is that something to ignore or make more robust?
##########
File path: src/CMakeLists.txt
##########
@@ -13,3 +13,19 @@ if(BUILD_TESTING)
add_subdirectory(test)
add_subdirectory(examples/cpp)
endif()
+
+option(BUILD_SITE "Build log4cxx website" OFF)
Review comment:
Why is this placed here instead of the upper level `CMakeLists.txt` containing `APACHE_MAINTAINER` already? Seems to make more sense to me to have both near by each other.
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
Review comment:
I suggest adding some indentation here to make things more readable, like you did with Doxygen already.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r555893325
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
+include(CPack)
+
+add_custom_target( dist
Review comment:
thanks, found it with a little help:
https://devblogs.microsoft.com/cppblog/cmake-support-in-visual-studio-targets-view-single-file-compilation-and-cache-generation-settings/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
rm5248 commented on pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#issuecomment-757587051
> Things look good in general to me, though I have some minor suggestions and questions.
>
> How do you plan to handle obsolete parts of ANT, MVN etc.? With an additional PR or make it part of this or ...?
I was going to do that as a separate PR, as I was only focusing on the website building with this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
rm5248 commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r554657944
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
Review comment:
Yes, I do believe that is part of how Visual Studio configures things - I will update that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
rm5248 commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r554660578
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
+include(CPack)
+
+add_custom_target( dist
+ COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} -- package_source
+ COMMAND ${CMAKE_COMMAND} -E sha512sum "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}.tar.gz" > "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}.tar.gz.sha512"
Review comment:
The 'sha512sum' is actually a built-in CMake command: https://cmake.org/cmake/help/latest/manual/cmake.1.html#run-a-command-line-tool
As for the `gpg`, since that's really only needed for us as Apache maintainers, we could either ignore it, or we could make it somewhat more robust and have a separate 'sign' target. This just makes it very easy to do it all at once.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#issuecomment-757520874
How to enable generating the SITE:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 merged pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
rm5248 merged pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
rm5248 commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r554659837
##########
File path: CMakeLists.txt
##########
@@ -103,6 +105,25 @@ foreach(varName HAS_STD_LOCALE HAS_ODBC HAS_MBSRTOWCS HAS_WCSTOMBS HAS_FWIDE
endif()
endforeach()
+#
+# Package and sign if Apache maintainer
+#
+option(APACHE_MAINTAINER "Apache maintainer" OFF)
+if(APACHE_MAINTAINER)
+set(CPACK_SOURCE_PACKAGE_FILE_NAME "apache-log4cxx-${LOG4CXX_RELEASE_VERSION}")
+set(CPACK_SOURCE_GENERATOR "TGZ;ZIP")
+set(CPACK_SOURCE_IGNORE_FILES ".git/;build/")
+include(CPack)
+
+add_custom_target( dist
Review comment:
At least on *nix, this creates a 'make dist' target. With Visual Studio, you need to switch to 'CMake Targets View' in the Solution Explorer. You can then run the target from there.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
rm5248 commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r555406781
##########
File path: src/CMakeLists.txt
##########
@@ -13,3 +13,19 @@ if(BUILD_TESTING)
add_subdirectory(test)
add_subdirectory(examples/cpp)
endif()
+
+option(BUILD_SITE "Build log4cxx website" OFF)
Review comment:
Fixed with the latest commit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] rm5248 commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
rm5248 commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r554661934
##########
File path: src/CMakeLists.txt
##########
@@ -13,3 +13,19 @@ if(BUILD_TESTING)
add_subdirectory(test)
add_subdirectory(examples/cpp)
endif()
+
+option(BUILD_SITE "Build log4cxx website" OFF)
Review comment:
Some of the options are spread over different CMake files. This seemed like the most logical place to put it at the time, but perhaps not. We could:
1. Put at top-level CMakeLists
2. Keep where it is
3. Create a new CMakeLists in src/site that contains this logic
Any of these options make sense to me, though perhaps option 3 makes the most sense in terms of how the CMakeLists is currently structured(one CMakeLists per directory).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #49: Documentation updates
Posted by GitBox <gi...@apache.org>.
ams-tschoening commented on a change in pull request #49:
URL: https://github.com/apache/logging-log4cxx/pull/49#discussion_r554978113
##########
File path: src/CMakeLists.txt
##########
@@ -13,3 +13,19 @@ if(BUILD_TESTING)
add_subdirectory(test)
add_subdirectory(examples/cpp)
endif()
+
+option(BUILD_SITE "Build log4cxx website" OFF)
Review comment:
Agreed, your option 3 seems to make most sense.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org