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

node: Add accepted formats and examples for rpc and ws parameter desc #3622

Merged
merged 4 commits into from
Jan 5, 2024

Conversation

bingyuyap
Copy link
Contributor

PR for #3612

@bingyuyap bingyuyap force-pushed the bing/node-parameters-description branch 2 times, most recently from dd63001 to bc22a93 Compare January 3, 2024 07:57
@bingyuyap
Copy link
Contributor Author

bingyuyap commented Jan 3, 2024

str.into() conversions are infallible. Using str.try_into().unwrap() would be unnecessary, triggering unnecessary-fallible-conversions error.
See: https://rust-lang.github.io/rust-clippy/master/index.html#/unnecessary_fallible_conversions

@bingyuyap bingyuyap force-pushed the bing/node-parameters-description branch from bc22a93 to b7b3553 Compare January 3, 2024 15:50
@bingyuyap bingyuyap changed the base branch from main to bing/rust-linter-unnecessary-fallible-conversions January 3, 2024 15:51
Base automatically changed from bing/rust-linter-unnecessary-fallible-conversions to main January 3, 2024 16:39
@bingyuyap bingyuyap force-pushed the bing/node-parameters-description branch from b7b3553 to 80622a7 Compare January 3, 2024 17:21
…ription

Signed-off-by: bingyuyap <bingyu.yap.21@gmail.com>
@bingyuyap bingyuyap force-pushed the bing/node-parameters-description branch from 80622a7 to 616d209 Compare January 3, 2024 20:35
@bingyuyap bingyuyap marked this pull request as ready for review January 3, 2024 20:50
@panoel panoel requested a review from SEJeff January 3, 2024 21:35
node/cmd/guardiand/node.go Outdated Show resolved Hide resolved
panoel
panoel previously approved these changes Jan 3, 2024
Signed-off-by: bingyuyap <bingyu.yap.21@gmail.com>
@bingyuyap bingyuyap force-pushed the bing/node-parameters-description branch 2 times, most recently from be2d73d to 2ba4570 Compare January 3, 2024 22:57
Signed-off-by: bingyuyap <bingyu.yap.21@gmail.com>
@bingyuyap bingyuyap force-pushed the bing/node-parameters-description branch from 2ba4570 to ec8e38b Compare January 3, 2024 22:59
@bingyuyap bingyuyap requested a review from panoel January 4, 2024 05:38
@bruce-riley
Copy link
Contributor

Will there be a follow up to this PR? Because I think we should have some verification to enforce that the parameter is the right format (with ws/wss vs. http/https, etc).

@bingyuyap
Copy link
Contributor Author

Will there be a follow up to this PR? Because I think we should have some verification to enforce that the parameter is the right format (with ws/wss vs. http/https, etc).

Yes the follow up PR will be addressing this issue to make it fail-fast.

@bingyuyap
Copy link
Contributor Author

@bruce-riley This is the follow up PR to add verification

@bingyuyap bingyuyap merged commit cd579d0 into main Jan 5, 2024
22 checks passed
@bingyuyap bingyuyap deleted the bing/node-parameters-description branch January 5, 2024 16:35
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