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

Use Ipv4Addr::{LOCALHOST, UNSPECIFIED} constants #29813

Merged
merged 1 commit into from
Jan 23, 2023
Merged

Use Ipv4Addr::{LOCALHOST, UNSPECIFIED} constants #29813

merged 1 commit into from
Jan 23, 2023

Conversation

kevinji
Copy link
Contributor

@kevinji kevinji commented Jan 21, 2023

Problem

No problem, just some code cleanup.

Summary of Changes

Use the Ipv4Addr::{LOCALHOST, UNSPECIFIED} constants instead of directly using 127.0.0.1 and 0.0.0.0.

@mergify mergify bot added the community Community contribution label Jan 21, 2023
@mergify mergify bot requested a review from a team January 21, 2023 01:21
@mvines mvines added the CI Pull Request is ready to enter CI label Jan 21, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Jan 21, 2023
@mvines mvines added the CI Pull Request is ready to enter CI label Jan 22, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Jan 22, 2023
@mvines mvines added the automerge Merge this Pull Request automatically once CI passes label Jan 22, 2023
mvines
mvines previously approved these changes Jan 22, 2023
View reviewed changes
Copy link
Member

@mvines mvines left a comment

Choose a reason for hiding this comment

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

thanks, much nicer!

@steviez
Copy link
Contributor

steviez commented Jan 22, 2023

Agreed with mvines - use of the constant is great so thank you!

Not to punish a good deed with a follow-on request, but searching the repo, I see several more instances of [127, 0, 0, 1] in the following two files:
https://github.com/solana-labs/solana/blob/master/core/src/serve_repair.rs
https://github.com/solana-labs/solana/blob/master/faucet/src/faucet.rs

Any interest in updating those in this PR as well? No worries if not, beggars can't be choosers and we appreciate the contribution either way

@solana-grimes solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label Jan 23, 2023
@solana-grimes
Copy link
Contributor

😱 New commits were pushed while the automerge label was present.

@mergify mergify bot dismissed mvines’s stale review January 23, 2023 19:46

Pull request has been modified.

@kevinji
Copy link
Contributor Author

kevinji commented Jan 23, 2023

@steviez Just updated the PR to include changes to those two files.

@mergify mergify bot requested a review from a team January 23, 2023 19:47
steviez
steviez approved these changes Jan 23, 2023
View reviewed changes
Copy link
Contributor

@steviez steviez left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for getting those last couple instances!

@steviez steviez added the CI Pull Request is ready to enter CI label Jan 23, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Jan 23, 2023
@steviez steviez merged commit dd92f22 into solana-labs:master Jan 23, 2023
@kevinji kevinji deleted the use-ipv4-constants branch January 23, 2023 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steviez steviez steviez approved these changes

@mvines mvines mvines left review comments

Assignees
No one assigned
Labels
community Community contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kevinji @steviez @solana-grimes @mvines