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

chore: improve code docs #28

Merged
merged 17 commits into from
Feb 5, 2024
Merged

chore: improve code docs #28

merged 17 commits into from
Feb 5, 2024

Conversation

BeroBurny
Copy link
Collaborator

Proposal for JS docs to make it simpler with links that can be used for more

image

like in this case, the user can click on NetworkArguments and will be redirected to a md file inside the git repository where you can read more about it

@BeroBurny
Copy link
Collaborator Author

it is in draft for discussion, it can be merged but is still VIP till we figure out all APIs

@BeroBurny BeroBurny changed the title chore: improve jsDocs chore: improve code docs Jan 22, 2024
Copy link
Member

@mpetrunic mpetrunic left a comment

Choose a reason for hiding this comment

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

Can you check how hardhat plugin docs work: https://hardhat.org/hardhat-runner/plugins/nomicfoundation-hardhat-ethers

is that just readme or?

* Deploys a contract to multiple blockchain networks.
*
* @param contractName - The name of the contract to be deployed.
* @param networkArgs - An object mapping network identifiers to their deployment arguments. Each network can have unique settings for the deployment. See [NetworkArguments]{@link https://github.com/ChainSafe/hardhat-plugin-multichain-deploy/docs/plugin/NetworkArguments}.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param networkArgs - An object mapping network identifiers to their deployment arguments. Each network can have unique settings for the deployment. See [NetworkArguments]{@link https://github.com/ChainSafe/hardhat-plugin-multichain-deploy/docs/plugin/NetworkArguments}.
* @param networkArgs - An object mapping network identifiers to their constructor arguments. Each network can have unique settings for the deployment. See [NetworkArguments]{@link https://github.com/ChainSafe/hardhat-plugin-multichain-deploy/docs/plugin/NetworkArguments}.

Copy link
Member

Choose a reason for hiding this comment

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

explain what is the network identifier. Give example for one network

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The example will be provided in the example section to reduce clutter parameters section.
Note it is deployment arguments as objects can contain constructor params and init params

```ts
interface NetworkArgument<Abi extends ContractAbi = any> {
args: ContractConstructorArgs<Abi>;
initData?: string;
Copy link
Member

Choose a reason for hiding this comment

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

explain initData?

/**
* Options used for contract deployment
*
* @property [salt] - Indicates whether salt is present, as Uint8Array or HexString.
Copy link
Member

Choose a reason for hiding this comment

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

explain what salt does :)

* Options used for contract deployment
*
* @property [salt] - Indicates whether salt is present, as Uint8Array or HexString.
* @property [isUniquePerChain] - Indicates whether contract address is uniq per deployment.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @property [isUniquePerChain] - Indicates whether contract address is uniq per deployment.
* @property [isUniquePerChain] - Indicates whether contract address should be same or different on each chain

packages/plugin/src/types.ts Outdated Show resolved Hide resolved
@mpetrunic
Copy link
Member

is this ready @BeroBurny ?

@BeroBurny
Copy link
Collaborator Author

Can you check how hardhat plugin docs work: https://hardhat.org/hardhat-runner/plugins/nomicfoundation-hardhat-ethers

is that just readme or?

It is just readme.

is this ready @BeroBurny ?

It is not. Have some API changes incoming and some things are unfinished.

# Conflicts:
#	packages/plugin/src/MultichainHardhatRuntimeEnvironmentField.ts
#	packages/plugin/src/types.ts
@BeroBurny BeroBurny marked this pull request as ready for review February 2, 2024 14:09
docs/plugin/NetworkArguments.md Show resolved Hide resolved
Comment on lines 44 to 47
networks: {
sepolia: { ... },
goerli: { ... }
},
Copy link
Contributor

@irubido irubido Feb 2, 2024

Choose a reason for hiding this comment

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

we only use networks for origin chain id, we should say in docs that destination networks are not mandatory to be listed here

## Usages

### TODO
## Usage
Copy link
Member

Choose a reason for hiding this comment

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

could you add example how to use it with local network?

@BeroBurny BeroBurny requested a review from irubido February 5, 2024 10:28
@BeroBurny BeroBurny merged commit 9c39669 into master Feb 5, 2024
2 checks passed
@BeroBurny BeroBurny deleted the beroburny/improve-js-docs branch February 5, 2024 13:27
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.

3 participants