Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(contracts): setup solidity docgen and configure it to work on docusaurus #870

Merged
merged 13 commits into from
Dec 12, 2023

Conversation

ctrlc03
Copy link
Collaborator

@ctrlc03 ctrlc03 commented Dec 4, 2023

fix #770

This PR adds support for Solidity Docgen as well as fixes syntax errors in current NatSpec comments (note it does not enhance the comments as that is to be worked on #843). Support for hosting the generated content on Docusaurus is also added

@ctrlc03 ctrlc03 self-assigned this Dec 4, 2023
@ctrlc03 ctrlc03 requested a review from samajammin December 4, 2023 22:38
Copy link
Member

@samajammin samajammin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Looks good overall but added some change requests.

@ctrlc03 ctrlc03 requested a review from samajammin December 5, 2023 17:42
@0xmad 0xmad changed the title docs(contracts) - setup solidity docgen and configure it to work on docusaurus docs(contracts): setup solidity docgen and configure it to work on docusaurus Dec 5, 2023
@ctrlc03 ctrlc03 force-pushed the docs/contracts branch 2 times, most recently from 2a1fc22 to c3450b3 Compare December 6, 2023 15:10

This comment was marked as outdated.

Copy link

netlify bot commented Dec 6, 2023

Deploy Preview for maci-typedoc ready!

Name Link
🔨 Latest commit 3101a60
🔍 Latest deploy log https://app.netlify.com/sites/maci-typedoc/deploys/657832b3dc3d090008950453
😎 Deploy Preview https://deploy-preview-870--maci-typedoc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

.github/workflows/gh-pages.yml Outdated Show resolved Hide resolved
@ctrlc03 ctrlc03 force-pushed the docs/contracts branch 5 times, most recently from d2ea1f2 to 9bd1e45 Compare December 7, 2023 11:06
@ctrlc03
Copy link
Collaborator Author

ctrlc03 commented Dec 11, 2023

I have some thoughts for the typedocs and soliditydocs, but not really related to this PR. Try to put some thoughts here though, if it's not a proper place, then we could move to open a new discussion thread, or somewhere else.

First of all, I wanna ask that if there's version difference between different versions? For example, the circuits.md file is different in v0.x and v1.x. Will type docs and solidity docs also have this difference? Such as changing type from Int to BigInt for some arguments in different versions. If so, I would suggest to have this kind of document structure: 截圖 2023-12-11 下午5 09 13 Also it's clearer for me that both typedoc and soliditydocs are belong to Documents, instead of separating them on the navbar.

Second, I also suggest to split PR into docgen related one and NatSpec one.

Last is I think the netlify is not displaying correct website?

@kittybest For v0.x vs v1.x - we will continue to host v0.x docs (legacy) but do not intend to update them or touch them at all. These are just a reference of how MACI v.0x works (though we do not support it anymore). Moving forward, all new content will only be inside v.1x or newer versions. TLDR no typedoc nor natspec docs for v.0x

Sure we can put them in the documents folder as highlighted by that image, though we will not be making typedoc related changes on this PR, but please feel free to open one yourself to move them.

While I agree that the natspec could have been moved to a separate one (which is also coming today), some of the tags were broken and were preventing docgen to complete. Would it be ok to keep it like this for now, just change the folder where the docs are hosted?

Not sure what you mean by not displaying the correct website? what is wrong about it?
Currently: Netlify: preview only - GH pages with custom domain: deploys on pushes to dev

@samajammin
Copy link
Member

@ctrlc03 FYI looks like builds are failing in Netlify:

6:09:24 AM: $ ./.github/scripts/website.sh
6:09:24 AM: bash: ./.github/scripts/website.sh: No such file or directory
6:09:24 AM: ​
6:09:24 AM: "build.command" failed                                        
6:09:24 AM: ────────────────────────────────────────────────────────────────
6:09:24 AM: ​
6:09:24 AM:   Error message
6:09:24 AM:   Command failed with exit code 127: ./.github/scripts/website.sh (https://ntl.fyi/exit-code-127)
6:09:24 AM: ​
6:09:24 AM:   Error location
6:09:24 AM:   In Build command from Netlify app:
6:09:24 AM:   ./.github/scripts/website.sh
6:09:24 AM: ​
6:09:24 AM:   Resolved config
6:09:24 AM:   build:
6:09:24 AM:     command: ./.github/scripts/website.sh
6:09:24 AM:     commandOrigin: ui
6:09:24 AM:     environment:
6:09:24 AM:       - REVIEW_ID
6:09:24 AM:     publish: /opt/build/repo/website/build
6:09:24 AM:     publishOrigin: ui
6:09:25 AM: Failed during stage "building site": Build script returned non-zero exit code: 2
6:09:25 AM: Build failed due to a user error: Build script returned non-zero exit code: 2
6:09:25 AM: Failing build: Failed to build site
6:09:25 AM: Finished processing build request in 19.439s

Is Netlify able to access that script? 🤔

@ctrlc03
Copy link
Collaborator Author

ctrlc03 commented Dec 11, 2023

@ctrlc03 FYI looks like builds are failing in Netlify:

6:09:24 AM: $ ./.github/scripts/website.sh
6:09:24 AM: bash: ./.github/scripts/website.sh: No such file or directory
6:09:24 AM: ​
6:09:24 AM: "build.command" failed                                        
6:09:24 AM: ────────────────────────────────────────────────────────────────
6:09:24 AM: ​
6:09:24 AM:   Error message
6:09:24 AM:   Command failed with exit code 127: ./.github/scripts/website.sh (https://ntl.fyi/exit-code-127)
6:09:24 AM: ​
6:09:24 AM:   Error location
6:09:24 AM:   In Build command from Netlify app:
6:09:24 AM:   ./.github/scripts/website.sh
6:09:24 AM: ​
6:09:24 AM:   Resolved config
6:09:24 AM:   build:
6:09:24 AM:     command: ./.github/scripts/website.sh
6:09:24 AM:     commandOrigin: ui
6:09:24 AM:     environment:
6:09:24 AM:       - REVIEW_ID
6:09:24 AM:     publish: /opt/build/repo/website/build
6:09:24 AM:     publishOrigin: ui
6:09:25 AM: Failed during stage "building site": Build script returned non-zero exit code: 2
6:09:25 AM: Build failed due to a user error: Build script returned non-zero exit code: 2
6:09:25 AM: Failing build: Failed to build site
6:09:25 AM: Finished processing build request in 19.439s

Is Netlify able to access that script? 🤔

there was a name mismatch, fixed now on our side.

let fileContent = fs.readFileSync(absolutePath, "utf8");

// Remove the "# Solidity API" line
fileContent = fileContent.replace("# Solidity API\n", "");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

import fs from "fs";
import path from "path";

const solidityDocDir = path.resolve(__dirname, "../../versioned_docs/version-v1.x/natspec-docs");
Copy link
Member

@samajammin samajammin Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest maybe solidity-api or contracts-api or solidity-docs instead of natspec-docs. Subjective take but I suspect many folks don't know what NatSpec is & may get confused.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's also a way to specify the display name of the directory (e.g. to "Solidity docs" or "Solidity API") 🤔

Image 2023-12-11 at 21 44 46

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can accomplish that with a custom sidebar, can do that in a later PR if you want - we can directly specify order and name there

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can accomplish that with a custom sidebar, can do that in a later PR if you want - we can directly specify order and name there

Sweet, love that idea!

import path from "path";

const solidityDocDir = path.resolve(__dirname, "../../versioned_docs/version-v1.x/natspec-docs");
const sourceDir = path.resolve(__dirname, "../../../contracts/docs");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment that we pull generated docs from the contracts package?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

Copy link
Member

@samajammin samajammin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! A couple comments but no blockers.

I didn't scrutinize the contract comments closely but website looks great & all pages are searchable! 🎉

Copy link
Member

@baumstern baumstern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thanks!

@ctrlc03 ctrlc03 merged commit 85b0b82 into dev Dec 12, 2023
11 of 12 checks passed
@ctrlc03 ctrlc03 deleted the docs/contracts branch December 12, 2023 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better documentation in smart contracts and solidity docs
4 participants