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.

Sign up for GitHub

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

Jump to bottom

Separated docs by components, added API docs #136

Merged
merged 6 commits into from
Jan 16, 2024
Merged

Separated docs by components, added API docs #136

merged 6 commits into from
Jan 16, 2024

Conversation

polydez
Copy link
Contributor

@polydez polydez commented Jan 10, 2024 •
edited
Loading

Hi there,
I've improved README by adding prerequisites, making separated page for each component and API documentation.

@polydez polydez requested review from bobbinth and phklive January 10, 2024 16:39
@polydez polydez mentioned this pull request Jan 10, 2024
Closed
@polydez polydez marked this pull request as ready for review January 10, 2024 16:43
@phklive
Copy link
Contributor

phklive commented Jan 10, 2024 •
edited
Loading

Hi @polydez,

I'm currently reviewing your contributions.

I wanted to share some guidelines with you for future reference on how we handle code contributions in Miden:

  1. Clone the Global Repository: Instead of forking, directly clone the repository to your local machine.

  2. Branch Naming Convention: When you start working on something new, create a branch named after yourself followed by a brief description of your work. Example: polydez-node-docs-improvement.

  3. Submitting a Pull Request (PR): Once you're ready, push your changes and submit a pull request to the global repository.

  4. PR Review and Merge: The team will review your PR. Upon approval, it will be merged into the main codebase.

To be confirmed with @bobbinth but I believe that you could keep it as it is for now and only follow this method for the next PR's.

Feel free to reach out if you have any questions. Thanks for being part of our team!

phklive
phklive requested changes Jan 10, 2024
View reviewed changes
Copy link
Contributor

@phklive phklive 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, this adds needed documentation.

Added some comments to make the documents more readable and correct some typos.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
block-producer/README.md Outdated
@@ -0,0 +1,50 @@
# Block Producer

**Block producer** accepts transactions from the RPC component, creates blocks containing those transactions, and
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend a change to:

The Block producer receives transactions from the RPC component, processes them, creates block containing those transactions before sending created blocks to the store.

Block Producer is one of components of the Miden node.

block-producer/README.md Outdated

## Architecture

The Miden node is still under heavy development and current architecture is subject of change.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to simple TODO

store/README.md Outdated
cargo install --path store
```

In order to run Store, you must provide genesis file. To generate genesis file you need to use [Miden node](../README.md#generating-the-genesis-file)'s `make-genesis` command.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to:

In order to run the Store, you must provide a genesis file. To generate a genesis file you will need to use Miden node's make-genesis command.

You will also need to provide a configuration file. We have an example config file in store-example.toml.

store/README.md Outdated

## API

**Store** serves connections using [gRPC protocol](https://grpc.io) on a port, set in configuration file. Here is a brief
Copy link
Contributor

Choose a reason for hiding this comment

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

Start sentence with The, add the after using.

add the previously mentioned before configuration file

rpc/README.md Outdated

## API

**RPC** serves connections using [gRPC protocol](https://grpc.io) on a port, set in configuration file. Here is a brief
Copy link
Contributor

Choose a reason for hiding this comment

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

Start sentence with The, add the after using.

add the previously mentioned before configuration file

block-producer/README.md Outdated

## API

**Block Producer** serves connections using [gRPC protocol](https://grpc.io) on a port, set in configuration file. Here is a brief
Copy link
Contributor

Choose a reason for hiding this comment

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

Start sentence with The, add the after using.

add the previously mentioned before configuration file

rpc/README.md Outdated
cargo install --path rpc
```

To run the RPC you'll need to provide a configuration file. We have an example config file in [rpc-example.toml](rpc-example.toml).
Copy link
Contributor

Choose a reason for hiding this comment

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

change you'll to you will

bobbinth
bobbinth approved these changes Jan 10, 2024
View reviewed changes
Copy link
Contributor

@bobbinth bobbinth 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! Thank you! I left some comments inline. Once these and @phklive's comments are addressed, we can merge.

rpc/README.md Outdated Show resolved Hide resolved
rpc/README.md Outdated Show resolved Hide resolved
rpc/README.md Outdated
Comment on lines 62 to 64
### SyncState

State synchronization request.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would write a small paragraph on how this is intended to be used as it may not be super obvious. There is some background info in #38 and #43, but I also kind of summarize it in 0xPolygonMiden/miden-client#63 (comment).

rpc/README.md Outdated Show resolved Hide resolved
store/README.md Show resolved Hide resolved
store/README.md Outdated Show resolved Hide resolved
store/README.md Outdated Show resolved Hide resolved
store/README.md Outdated Show resolved Hide resolved
store/README.md Outdated Show resolved Hide resolved
block-producer/README.md Outdated Show resolved Hide resolved
@bobbinth bobbinth linked an issue Jan 11, 2024 that may be closed by this pull request
Add API specifications to the documentation of miden-node #129
Closed
@polydez
Copy link
Contributor Author

polydez commented Jan 15, 2024

@bobbinth, I have finished editing, please, take a look!

@bobbinth
Copy link
Contributor

Looks good! Thank you!

@bobbinth bobbinth merged commit 4d626c3 into 0xPolygonMiden:main Jan 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phklive phklive phklive requested changes

@bobbinth bobbinth bobbinth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add API specifications to the documentation of miden-node
3 participants
@polydez @phklive @bobbinth