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 information about sozo arguments priorities #176 #269

Conversation

RolandoDrRobot
Copy link
Contributor

@RolandoDrRobot RolandoDrRobot commented May 13, 2024

This the issue #176

Sozo is being reworked to have these priorities:

  • CLI arguments always takes precedence.
  • Then the environment variables.
  • Finally, anything found inside the scarb manifest.

We may add a section on that on the book once this PR is merged dojoengine/dojo#1405 to bring better understanding for the user to the possibilities around configurations.

Captura de pantalla 2024-05-12 a la(s) 6 47 55 p  m

Copy link

vercel bot commented May 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
book ✅ Ready (Inspect) Visit Preview Jun 2, 2024 5:50pm

@RolandoDrRobot
Copy link
Contributor Author

Hello @glihm 👋 could you check If this is going on good path? thanks!
Here you can reach me If wanna dm me:

Discord: roloxworld
Telegram: roloxworld

@lambda-0x lambda-0x self-requested a review May 13, 2024 07:30
@lambda-0x
Copy link
Collaborator

hello @RolandoDrRobot, thank you for the contribution. I will review this ASAP.

Copy link
Contributor

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Thanks for the PR here @RolandoDrRobot! We may also show some examples on how to use a keystore file instead of private key for instance. To ensure users know about those alternatives.

docs/pages/toolchain/sozo/common-options/configurations.md Outdated Show resolved Hide resolved
Comment on lines 21 to 22
- `account(provider, env_metadata)`: Creates a StarkNet account using the specified options. Takes into account the configuration priorities to determine the account address and other settings.
- `account_address(env_metadata)`: Returns the StarkNet account address. Takes into account the configuration priorities.
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't those inside the sozo code? Not sure if a sozo user should know about this technical specificity. Or what was the idea behind this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just documenting everything I saw in the code file https://github.com/lambda-0x/dojo/blob/9bf0a0894eadb7052fab586886ee02ba6beae351/crates/sozo/src/commands/options/account.rs but you are right, people may not need this, removing It

docs/pages/toolchain/sozo/common-options/configurations.md Outdated Show resolved Hide resolved
docs/pages/toolchain/sozo/common-options/configurations.md Outdated Show resolved Hide resolved
@@ -35,3 +35,9 @@ The configuration system follows the following priority order:
```bash
sozo --rpc-url http://localhost:7474/
```

### Use a keystore file for account configuration
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this correct? what u were meaning? otherwise could you provide me an example or where to get It? thanks @glihm @lambda-0x

@RolandoDrRobot
Copy link
Contributor Author

@lambda-0x @glihm Hello guys 👋 is this still active? let me know If wanna add something else to PR

@glihm
Copy link
Contributor

glihm commented May 29, 2024

@lambda-0x @glihm Hello guys 👋 is this still active? let me know If wanna add something else to PR

Sorry for the delay man the notification went of my radar, appreciate your ping here. @lambda-0x has some feedback he will made and then ready to merge.

@RolandoDrRobot
Copy link
Contributor Author

@glihm thank you 🙏

@lambda-0x lambda-0x force-pushed the ra-book-176-Add-information-about-sozo-arguments-priorities branch from eee32c4 to 4c1748a Compare June 2, 2024 17:44
@lambda-0x lambda-0x force-pushed the ra-book-176-Add-information-about-sozo-arguments-priorities branch from 5f92f87 to 339b1d5 Compare June 2, 2024 17:49
@lambda-0x
Copy link
Collaborator

@RolandoDrRobot sorry for the delay on this, and thank you for your contribution!

@lambda-0x lambda-0x merged commit 2a42fda into dojoengine:main Jun 2, 2024
1 check passed
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

3 participants