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

Improve TypeDoc by improving the layout and adding some comments #541

Merged
merged 8 commits into from Apr 8, 2021

Conversation

mehtaphysical
Copy link
Contributor

@mehtaphysical mehtaphysical commented Mar 26, 2021

This PR adds comments and updates the TypeDoc dependency to improve the generated documentation. Contributes to #483.

It also bring in the typedoc-neo-theme which helps improve readability of the generated docs. We are a bit blocked by this outstanding issue: google/typedoc-neo-theme#67.

Here is a link to a preview: http://dweb.link/ipfs/QmenzPfZTuS17LHYCYsiFFFC855xBG8xzqzCYUHtXi8Pnf

package.json Outdated
@@ -22,7 +22,8 @@
"mustache": "^4.0.0",
"node-fetch": "^2.6.1",
"text-encoding-utf-8": "^1.0.2",
"tweetnacl": "^1.0.1"
"tweetnacl": "^1.0.1",
"typedoc-neo-theme": "^1.1.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

@mehtaphysical this should be in devDependencies together with typedoc

src/connect.ts Outdated
*
* To sign transactions you can also pass:
* 1. {@link ConnectConfig.keyStore}
* 2. {@link ConnectConfig.deps | ConnectConfig.deps.keyStore}
Copy link
Contributor

Choose a reason for hiding this comment

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

let's just skip deps from the docs, it should be deprecated

*
* To sign transactions you can also pass:
* 1. {@link ConnectConfig.keyStore}
* 2. {@link ConnectConfig.deps | ConnectConfig.deps.keyStore}
Copy link
Contributor

Choose a reason for hiding this comment

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

let's just skip deps from the docs, it should be deprecated

Copy link
Contributor

@vgrichina vgrichina left a comment

Choose a reason for hiding this comment

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

@mehtaphysical looks good to me, but need to fix minor issues with devDependencies and deps

Copy link
Contributor

@thisisjoshford thisisjoshford left a comment

Choose a reason for hiding this comment

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

This is a great direction to take with improving the TypeDocs for NAJ.

src/connect.ts Outdated
@@ -47,16 +46,29 @@ export async function connect(config: ConnectConfig): Promise<Near> {
// TODO: Only load key if network ID matches
const keyPair = accountKeyFile[1];
const keyPathStore = new InMemoryKeyStore();
await keyPathStore.setKey(config.networkId, accountKeyFile[0], keyPair);
await keyPathStore.setKey(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: feels like this and following reformatting isn't related to content of PR (generally best avoid making unnecessary changes to not mess with git blame) + seems to use vertical space a bit too agressively

Copy link
Contributor

@vgrichina vgrichina left a comment

Choose a reason for hiding this comment

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

Looks good overall, but need to resolve conflicts

@mehtaphysical
Copy link
Contributor Author

conflicts fixed. Can i merge this?

@mikedotexe
Copy link
Contributor

Awesome work, merging. This is exactly what we need to do for near-sdk-rs

@mikedotexe mikedotexe merged commit 71920e0 into master Apr 8, 2021
@mikedotexe mikedotexe deleted the feat/improve-docs branch April 8, 2021 20:46
@thisisjoshford thisisjoshford mentioned this pull request May 21, 2021
4 tasks
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.

None yet

4 participants