Problem: Missing cpp library documents for developers #354

damoncro · 2022-04-26T09:45:43Z

Close:

Problem: Missing cpp library documents for developers #329

Solutions:

Add Erc20 doc comments examples
Add testnet tests
Add Doxygen, Sphinx, GitBook, mdbook supports
Generate the cpp documents with nix
Add github action docs.yml

damoncro · 2022-05-04T09:18:13Z

Added PR on cxx dtolnay/cxx#1048

damoncro · 2022-05-05T11:23:44Z

About gitbook, some problems on the graceful package, solution is here:

https://stackoverflow.com/questions/64211386/gitbook-cli-install-error-typeerror-cb-apply-is-not-a-function-inside-graceful

damoncro · 2022-05-05T11:27:27Z

Besides, https://github.com/matusnovak/doxybook2/ does not support mdbook, so mdbook may have some minor issues (some functions name are not clickable). May need some times to tweak.

So far, the work flow: rust -> c++ -> doxygen -> doxybook2 -> gitbook is the best, although gitbook is deprecated.

tomtau

ok to merge, we'll see if the patch to cxx is accepted

tomtau · 2022-05-06T02:52:35Z

bindings/cpp/Cargo.toml

@@ -19,6 +19,11 @@ tokio = { version = "1", features = ["rt"] }
 [build-dependencies]
 cxx-build = "1"

+# TODO Wait until https://github.com/dtolnay/cxx/pull/1048 is merged or similar solution is found
+cxx-build-with-doxygen = { git = "https://github.com/damoncro/cxx.git", rev = "42b011184b6d5a593cd7513edb1d554e0f086a0f", package = "cxx-build", optional = true }


Suggested change

cxx-build-with-doxygen = { git = "https://github.com/damoncro/cxx.git", rev = "42b011184b6d5a593cd7513edb1d554e0f086a0f", package = "cxx-build", optional = true }

cxx-build-with-doxygen = { git = "https://github.com/crypto-com/cxx.git", rev = "42b011184b6d5a593cd7513edb1d554e0f086a0f", package = "cxx-build", optional = true }

tomtau · 2022-05-06T03:01:31Z

bindings/cpp/build.rs

+    #[cfg(not(feature = "doxygen"))]
    cxx_build::bridges(BRIDGES)
        .flag_if_supported("-std=c++11")
        .compile("defi_wallet_core");

+    #[cfg(feature = "doxygen")]
+    cxx_build_with_doxygen::bridges(BRIDGES)
+        .flag_if_supported("-std=c++11")


should this be feature-guarded here? is there a situation here why one wouldn't want the build to be with doxygen-style comments? even for the headers, I guess IDEs may display them better if with Doxygen-style comments?

Now, it is still a workaround, enabling doxygen feature in Windows CI build will fail.

See https://github.com/crypto-com/defi-wallet-core-rs/runs/6304031148?check_suite_focus=true

So for the document generation, generate it when necessary.

ok, maybe the comment can be added to explain... and perhaps instead of a feature, guard it by the target platform (windows / not-windows)?

Let me fix the windows ci... I found it was not difficult.

tomtau · 2022-05-06T03:03:39Z

bindings/cpp/src/contract.rs

@@ -18,6 +18,14 @@ fn new_erc20(contract_address: String, web3api_url: String, chain_id: u64) -> ff
 }
 impl ffi::Erc20 {
    /// Returns the decimal amount of tokens owned by `account_address`.
+    /// # Examples


not sure if there's this functionality similar to rustdoc in doxygen... but are the examples compiled to ensure that they are not out of date? (just like with rustdoc)

let me double check

Follow in #394

tomtau · 2022-05-06T03:05:32Z

docs/cpp/Doxyfile

+# pixels and the maximum width should not exceed 200 pixels. Doxygen will copy
+# the logo to the output directory.
+
+PROJECT_LOGO           =


I haven't read all the config options, but I guess these can later be tweaked by Leslie or others?

Most configures are not relevant, doxygen only provides the xml files to further generating markdown files. Lesile should tweak the configures in the final gitbook markdown files.

After discussed in the gamefi weekly meeting, we should upload the markdowns to a new repository and then git sync to gitbook platform.

Further adjustments would be made in markdowns.

Follow up in #396

tomtau · 2022-05-06T03:07:22Z

docs/cpp/README.md

+## Doxygen
+### Installation
+
+    brew install doxygen


the build instructions here may not be reproducible... so perhaps good to mention the minimum versions to use or provide a Nix environment?

ok, let me setup it.

Close: - #329 Solutions: - Add Eth doc comments examples - Add Erc20 doc comments examples - Add Erc721 doc comments examples - Add Erc1155 doc comments examples - Add Doxygen, Sphinx, GitBook, mdbook supports

damoncro · 2022-05-11T08:13:06Z

Outstanding issue: #395

leejw51crypto

lgtm

damoncro marked this pull request as draft April 26, 2022 09:45

damoncro force-pushed the feat/cpp_documents_for_developers branch 2 times, most recently from ac85418 to e1e6a34 Compare May 3, 2022 04:03

damoncro force-pushed the feat/cpp_documents_for_developers branch from a2e3699 to b15378b Compare May 5, 2022 10:51

damoncro marked this pull request as ready for review May 5, 2022 10:51

damoncro requested review from leejw51crypto, tomtau, stevenatcrypto, lezzokafka and ricky321q May 5, 2022 10:51

tomtau reviewed May 6, 2022

View reviewed changes

damoncro force-pushed the feat/cpp_documents_for_developers branch 2 times, most recently from 75c2d46 to 9011fce Compare May 10, 2022 03:24

damoncro added 12 commits May 11, 2022 13:22

Problem: Missing cpp library documents for developers

82bdd71

Close: - #329 Solutions: - Add Eth doc comments examples - Add Erc20 doc comments examples - Add Erc721 doc comments examples - Add Erc1155 doc comments examples - Add Doxygen, Sphinx, GitBook, mdbook supports

Update Cargo.lock

fe499b7

Apply nix to build cpp docs

45d7f56

Guard cxx-build with target_os

16a13c3

Format shell.nix

a378c5b

Run git config --global core.symlinks true for cxx

6112731

Add docs.yml

966ec7d

Rename it as Cpp Documents

2f6d462

Open docs only in mac

8efdd4c

Fix the action naming

d26509c

Fix indent in Makefile

7328dfd

Support gitbook in nix

fd6a2d7

damoncro force-pushed the feat/cpp_documents_for_developers branch from 2c283f2 to fd6a2d7 Compare May 11, 2022 05:23

Run nixpkgs-fmt

aabf1bc

Only serve in mac

05ee4d9

damoncro requested review from tomtau, leejw51crypto, stevenatcrypto, lezzokafka and ricky321q and removed request for leejw51crypto, lezzokafka, stevenatcrypto and ricky321q May 11, 2022 08:05

tomtau requested a review from linfeng-crypto May 11, 2022 08:07

leejw51crypto approved these changes May 11, 2022

View reviewed changes

stevenatcrypto approved these changes May 11, 2022

View reviewed changes

tomtau merged commit d883192 into main May 12, 2022

tomtau deleted the feat/cpp_documents_for_developers branch May 12, 2022 01:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Problem: Missing cpp library documents for developers #354

Problem: Missing cpp library documents for developers #354

damoncro commented Apr 26, 2022 •

edited

Loading

damoncro commented May 4, 2022

damoncro commented May 5, 2022

damoncro commented May 5, 2022 •

edited

Loading

tomtau left a comment

tomtau May 6, 2022

damoncro May 10, 2022

damoncro May 10, 2022

tomtau May 6, 2022

damoncro May 10, 2022 •

edited

Loading

tomtau May 10, 2022 •

edited

Loading

damoncro May 10, 2022

damoncro May 10, 2022

tomtau May 6, 2022

damoncro May 10, 2022

damoncro May 11, 2022

tomtau May 6, 2022

damoncro May 10, 2022

damoncro May 11, 2022

tomtau May 6, 2022

damoncro May 10, 2022

damoncro May 11, 2022

damoncro commented May 11, 2022

leejw51crypto left a comment

	cxx-build-with-doxygen = { git = "https://github.com/damoncro/cxx.git", rev = "42b011184b6d5a593cd7513edb1d554e0f086a0f", package = "cxx-build", optional = true }
	cxx-build-with-doxygen = { git = "https://github.com/crypto-com/cxx.git", rev = "42b011184b6d5a593cd7513edb1d554e0f086a0f", package = "cxx-build", optional = true }

Problem: Missing cpp library documents for developers #354

Problem: Missing cpp library documents for developers #354

Conversation

damoncro commented Apr 26, 2022 • edited Loading

damoncro commented May 4, 2022

damoncro commented May 5, 2022

damoncro commented May 5, 2022 • edited Loading

tomtau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damoncro May 10, 2022 • edited Loading

Choose a reason for hiding this comment

tomtau May 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damoncro commented May 11, 2022

leejw51crypto left a comment

Choose a reason for hiding this comment

damoncro commented Apr 26, 2022 •

edited

Loading

damoncro commented May 5, 2022 •

edited

Loading

damoncro May 10, 2022 •

edited

Loading

tomtau May 10, 2022 •

edited

Loading