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

cmd/clef: change --rpcport to --http.port and update docs #21318

Merged
merged 9 commits into from
Jul 14, 2020
Merged

cmd/clef: change --rpcport to --http.port and update docs #21318

merged 9 commits into from
Jul 14, 2020

Conversation

renaynay
Copy link
Contributor

@renaynay renaynay commented Jul 9, 2020

This PR updates the flag descriptions in the README of clef and also changes the rpcport flag to http.clefport (since the default is 8550 rather than 8545 as is the default for regular http).

QUESTION
Should we allow the legacy flag for port (rpcport) to still be valid and hide it when displaying flags with --help ?

@renaynay renaynay requested a review from holiman July 9, 2020 15:23
@fjl
Copy link
Contributor

fjl commented Jul 9, 2020

I think I prefer --http.port over --http.clefport. It's good to be consistent with geth here. The flag sets the port of the HTTP server. The default port should not determine the flag name.

@renaynay
Copy link
Contributor Author

make a new package, move all flag helpers cmd/internal/flags or something like that. Lots of duplicate code.

@renaynay renaynay requested a review from fjl July 13, 2020 11:20
holiman
holiman approved these changes Jul 13, 2020
View reviewed changes
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman holiman changed the title cmd/clef: updated flag descriptions, changed rpcport to http.clefport for consistency with other flags cmd/clef: updated flag descriptions, changed clef rpcport to http.port for consistency with other flags Jul 13, 2020
@fjl fjl changed the title cmd/clef: updated flag descriptions, changed clef rpcport to http.port for consistency with other flags cmd/clef: change --rpcport to --http.port and update docs Jul 14, 2020
@fjl fjl merged commit 5b081ab into ethereum:master Jul 14, 2020
@ricardolyn ricardolyn mentioned this pull request Feb 19, 2021
Merged
9 tasks
enriquefynn pushed a commit to enriquefynn/go-ethereum that referenced this pull request Mar 10, 2021
@renaynay @enriquefynn
cmd/clef: change --rpcport to --http.port and update flags in docs (e…
c943e6c 
…thereum#21318)
@gzliudan gzliudan mentioned this pull request Nov 25, 2024
Merged
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holiman holiman holiman approved these changes

@fjl fjl Awaiting requested review from fjl

Assignees
No one assigned
Labels
status:triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@renaynay @fjl @holiman