You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "silver-ymz (via GitHub)" <gi...@apache.org> on 2023/08/31 10:13:43 UTC

[GitHub] [incubator-opendal] silver-ymz opened a new pull request, #2988: docs(bindings/cpp): use doxygen to generate API docs

silver-ymz opened a new pull request, #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988

   (no comment)


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo merged pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] silver-ymz commented on a diff in pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on code in PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#discussion_r1312455784


##########
licenserc.toml:
##########
@@ -38,4 +38,5 @@ excludes = [
 
   # Third-party source files
   "scripts/merge_local_staging.sh",
+  "bindings/cpp/docs/doxygen-awesome.css",

Review Comment:
   completed in new 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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo commented on pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#issuecomment-1701215669

   > We just need single file of the repo, so I think it's a better way to just copy one file under our repo.
   
   No, not a good way from releases side.
   
   > Furthermore, I find that other repos also use this way, like wxWidgets, opencv 5.x.
   
   None of them are ASF projects. Although there are ways to declare an extra license for our repository, we should still avoid including content in this manner as it would complicate our release process.
   
   > We can use other way to install them like npm or FetchContent in cmake.
   
   I prefer to use the npm way mentioned in their README:
   
   ```shell
   cd your-project
   npm install https://github.com/jothepro/doxygen-awesome-css#v2.2.1 --save-dev
   
   ls -l node_module/@jothepro/doxygen-awesome-css
   ```


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] silver-ymz commented on pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#issuecomment-1700995291

   > Can we install them by npm during docs building?
   
   We can use other way to install them like `npm` or `FetchContent` in cmake. But this will make doc generating process become much complex. We just need single file of the repo, so I think it's a better way to just copy one file under our repo. Furthermore, I find that other repos also use this way, like wxWidgets, opencv 5.x.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] silver-ymz commented on pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#issuecomment-1701987489

   I move `awesome-doxygen` to build process. Cmake will download it to build dir first, rather than try finding it in docs dir.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#discussion_r1312454426


##########
licenserc.toml:
##########
@@ -38,4 +38,5 @@ excludes = [
 
   # Third-party source files
   "scripts/merge_local_staging.sh",
+  "bindings/cpp/docs/doxygen-awesome.css",

Review Comment:
   Please remove this.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] silver-ymz commented on pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#issuecomment-1702067982

   > Would you like to add this theme for c binding too?
   
   Yes. I'd like to do it.
   
   > And we should add actions to deploy them to opendal.apache.org.
   
   I'm writing github actions about test and docs now.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#discussion_r1312463666


##########
bindings/cpp/CMakeLists.txt:
##########
@@ -80,4 +80,25 @@ if(APPLE)
 endif()
 
 include(GoogleTest)
-gtest_discover_tests(opendal_cpp_test)
\ No newline at end of file
+gtest_discover_tests(opendal_cpp_test)
+
+# Documentation
+set(PROJECT_DOCUMENT_SOURCE ${CMAKE_SOURCE_DIR}/include ${CMAKE_SOURCE_DIR}/README.md)
+string(REPLACE ";" " " PROJECT_DOCUMENT_SOURCE "${PROJECT_DOCUMENT_SOURCE}")
+file(DOWNLOAD https://raw.githubusercontent.com/jothepro/doxygen-awesome-css/main/doxygen-awesome.css ${CMAKE_BINARY_DIR}/doxygen-awesome.css)

Review Comment:
   Please don't download from github raw content, use CDN instead, for example: 
   
   https://cdn.jsdelivr.net/gh/jothepro/doxygen-awesome-css@2.2.1/doxygen-awesome.min.css
   
   Benefits:
   
   - Pinned to version 2.2.1
   - Minimize on demand.



-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo commented on pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#issuecomment-1700957161

   > I use [Doxygen Awesome](https://github.com/jothepro/doxygen-awesome-css) to beautify the document generated by doxygen. I directly put its css file under `docs` dir. The css file has MIT license header. Could we add it to `licenserc.toml`?
   
   It's better not to include it in our repo directly. Can we isntall them by npm during docs building?


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] silver-ymz commented on a diff in pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on code in PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#discussion_r1312466198


##########
bindings/cpp/CMakeLists.txt:
##########
@@ -80,4 +80,25 @@ if(APPLE)
 endif()
 
 include(GoogleTest)
-gtest_discover_tests(opendal_cpp_test)
\ No newline at end of file
+gtest_discover_tests(opendal_cpp_test)
+
+# Documentation
+set(PROJECT_DOCUMENT_SOURCE ${CMAKE_SOURCE_DIR}/include ${CMAKE_SOURCE_DIR}/README.md)
+string(REPLACE ";" " " PROJECT_DOCUMENT_SOURCE "${PROJECT_DOCUMENT_SOURCE}")
+file(DOWNLOAD https://raw.githubusercontent.com/jothepro/doxygen-awesome-css/main/doxygen-awesome.css ${CMAKE_BINARY_DIR}/doxygen-awesome.css)

Review Comment:
   It looks great. updated in new 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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo commented on pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#issuecomment-1702084067

   Thanks a lot!


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] silver-ymz commented on pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "silver-ymz (via GitHub)" <gi...@apache.org>.
silver-ymz commented on PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#issuecomment-1700766294

   I use [Doxygen Awesome](https://github.com/jothepro/doxygen-awesome-css) to beautify the document generated by doxygen. I directly put its css file under `docs` dir. The css file has MIT license header. Could we add apache license before this file directly?


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-opendal] Xuanwo commented on pull request #2988: docs(bindings/cpp): use doxygen to generate API docs

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #2988:
URL: https://github.com/apache/incubator-opendal/pull/2988#issuecomment-1702064186

   Would you like to add this theme for c binding too? And we should add actions to deploy them to opendal.apache.org.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org