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

Add documentation for Stone Binaries and removed overlapping instruction from README #40

Merged
merged 8 commits into from
Oct 1, 2024

Conversation

CollinsC1O
Copy link
Contributor

This pull request includes the following updates:

Added Documentation for Stone Binaries:
- Created a new documentation page in the path docs/pages/install/binaries.md.
- This page provides detailed instructions on how to download and install Stone binaries based on different OS/architecture.
-  Added instructions on how to place binaries in system paths and configure system PATH variables.

Removed Overlapping Instructions from README:
- Cleaned up redundant sections in the README.md to avoid duplication with the new documentation.

This contribution makes the instructions clearer by moving the binary installation steps to a separate document and making the README.md clean and easier to read. Thanks

@CollinsC1O
Copy link
Contributor Author

hello @dmirgaleev I have implemented the required changes, please review for merging and let me know if to make any changes or improvement where necessary

source ~/.zshrc
```

4. Apply the changes by running:
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to do source ~/.bashrc twice (step 3 and 4)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hello @dmirgaleev. I will remove the redundant step right. Had network issues day but fixed now

README.md Outdated
@@ -85,11 +83,6 @@ docker pull ghcr.io/dipdup-io/stone-packaging/stone-prover:latest

### Creating and Verifying a Test Proof Using Docker

Clone the repository:
Copy link
Member

Choose a reason for hiding this comment

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

Let's download https://github.com/dipdup-io/stone-packaging?tab=readme-ov-file#download-binaries-for-x86_64 instead of cloning, we still need to clone the repo to create a verify a test proof

@CollinsC1O
Copy link
Contributor Author

Hello @dmirgaleev I have implemented the changes please review. thanks

Copy link
Member

@dmirgaleev dmirgaleev left a comment

Choose a reason for hiding this comment

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

Hey! In general, I would suggest to leave README.md as it was before (remove all your changes). And focus on the binaries.md. I think there we only need to think about the final step

After placing the binaries or updating the PATH, you can verify the installation by running:

```bash
cpu_air_prover --help
Copy link
Member

Choose a reason for hiding this comment

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

Does that work for you? What output do you get?

Copy link
Contributor Author

@CollinsC1O CollinsC1O Oct 1, 2024

Choose a reason for hiding this comment

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

hello @dmirgaleev it kinda worked but came up with some feedback which incudes a warning: 'SetUsageMessage() never called', just thought that since its not a critical error but a warning then it should still work. seems I will have to go around that and send in any update and progress

Copy link
Contributor Author

@CollinsC1O CollinsC1O Oct 1, 2024

Choose a reason for hiding this comment

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

just to mention that previously when i use the command source ~/.bashrc to reload my .bashrc it did come up with errors such as "line 102: syntax error near unexpected token `then'
" and "line 114: syntax error: unexpected end of file
". so had to go into my shell configuration file and correct the syntax error and made sure indentations are fixed correctly before it reloaded properly

README.md Outdated
@@ -25,6 +25,9 @@ Follow-up work:

## Usage Instructions

### Downloading Binaries
Copy link
Member

Choose a reason for hiding this comment

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

Let's leave it as it was before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

README.md Outdated
```bash
git clone https://github.com/dipdup-io/stone-packaging.git /tmp/stone-packaging
```
Note: You still need to clone the repo to create and verify a test proof.
Copy link
Member

Choose a reason for hiding this comment

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

Let's leave it as it was before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok will do that. implementing changes now

README.md Outdated
@@ -43,20 +46,14 @@ wget https://github.com/dipdup-io/stone-packaging/releases/latest/download/cpu_a

### Creating and Verifying a Test Proof Using Binaries

Clone the repository:
Copy link
Member

Choose a reason for hiding this comment

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

The same, we still need that. Please, try to reproduce the instructions you provided

@dmirgaleev
Copy link
Member

Resolves #23

@dmirgaleev dmirgaleev merged commit 0754edd into dipdup-io:master Oct 1, 2024
1 check failed
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.

2 participants