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

Display default values for EnvironmentOptions in katana cli #1861

Merged
merged 7 commits into from
Apr 25, 2024
Merged

Conversation

notV4l
Copy link
Collaborator

@notV4l notV4l commented Apr 22, 2024

fixes #1815

add extra logs at start

ENVIRONMENT
==================

| Chain ID            | 0x4b4154414e41
| Disable Fee         | false
| Disable Validate    | false
| Validate max steps  | 1000000
| Invoke max steps    | 1000000
| ETH Gas Price       | 1112701192
| STRK Gas Price      | 0

FORKING
==================
    
| Fork rpc url        | https://api.cartridge.gg/rpc/starknet
| Fork blocknumber    | 42

Copy link

codecov bot commented Apr 22, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 70.25%. Comparing base (d160479) to head (e35c4ef).
Report is 6 commits behind head on main.

Files Patch % Lines
bin/katana/src/args.rs 66.66% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1861      +/-   ##
==========================================
+ Coverage   70.14%   70.25%   +0.10%     
==========================================
  Files         315      313       -2     
  Lines       35607    35721     +114     
==========================================
+ Hits        24978    25097     +119     
+ Misses      10629    10624       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glihm glihm added the katana This issue is related to Katana label Apr 23, 2024
Copy link
Member

@kariy kariy left a comment

Choose a reason for hiding this comment

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

im not sure about the prints. i have some idea about displaying the config on startup and the config types are a bit messy right now, so would rather so would prefer if we dont print anything else for now.

bin/katana/src/args.rs Outdated Show resolved Hide resolved
bin/katana/src/args.rs Outdated Show resolved Hide resolved
bin/katana/src/args.rs Outdated Show resolved Hide resolved
bin/katana/src/args.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@notV4l
Copy link
Collaborator Author

notV4l commented Apr 24, 2024

im not sure about the prints. i have some idea about displaying the config on startup and the config types are a bit messy right now, so would rather so would prefer if we dont print anything else for now.

idk why its an issue to diplay more infos but up to you

@kariy
Copy link
Member

kariy commented Apr 25, 2024

im not sure about the prints. i have some idea about displaying the config on startup and the config types are a bit messy right now, so would rather so would prefer if we dont print anything else for now.

idk why its an issue to diplay more infos but up to you

Not really an issue with displaying the infos per se, but just wanting to be conservative about the stuff we add unless there's an actual demand for this feature. I was thinking of doing some rework on the node config first before doing any sort change on this side.

so tbh its really just me being picky :)

Copy link
Member

@kariy kariy left a comment

Choose a reason for hiding this comment

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

lgtm

@kariy kariy merged commit 61f511d into main Apr 25, 2024
11 of 12 checks passed
@kariy kariy deleted the katana_1815 branch April 25, 2024 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
katana This issue is related to Katana
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[katana] Display missing default values in command line --help
3 participants