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

fix(nodebuilder): Fix dns lookup #2332

Merged
merged 10 commits into from
Jun 13, 2023
Merged

fix(nodebuilder): Fix dns lookup #2332

merged 10 commits into from
Jun 13, 2023

Conversation

walldiss
Copy link
Member

@walldiss walldiss commented Jun 6, 2023 •
edited by Wondertan
Loading

Overview

Cleanup core ip parsing logic

Closes #2333

@walldiss walldiss added area:node Node kind:fix Attached to bug-fixing PRs labels Jun 6, 2023
@walldiss walldiss requested a review from renaynay as a code owner June 6, 2023 16:18
@walldiss walldiss self-assigned this Jun 6, 2023
Wondertan
Wondertan reviewed Jun 6, 2023
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Let's add unit test for the ValidateAddr with those two cases so we never come to this problem again

@Wondertan Wondertan mentioned this pull request Jun 6, 2023
Closed
@Wondertan
Copy link
Member

@walldiss, bump

@walldiss walldiss force-pushed the fix_dns_lookup branch 2 times, most recently from 95d3b26 to 0629a0f Compare June 7, 2023 16:34
Wondertan
Wondertan previously approved these changes Jun 8, 2023
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

We will get it on the 3rd attempt!

libs/utils/address.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Jun 8, 2023
View reviewed changes
libs/utils/address.go Outdated Show resolved Hide resolved
@Wondertan
Copy link
Member

@distractedm1nd, need you review here

@Wondertan
Copy link
Member

@walldiss, the unit test here is failing for some reason

distractedm1nd
distractedm1nd approved these changes Jun 13, 2023
View reviewed changes
Copy link
Collaborator

@distractedm1nd distractedm1nd left a comment

Choose a reason for hiding this comment

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

utACK

@walldiss walldiss enabled auto-merge (squash) June 13, 2023 11:18
@walldiss walldiss merged commit c41738a into celestiaorg:main Jun 13, 2023
@faddat faddat mentioned this pull request Feb 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@distractedm1nd distractedm1nd distractedm1nd approved these changes

@Wondertan Wondertan Wondertan approved these changes

@renaynay renaynay Awaiting requested review from renaynay renaynay is a code owner

@vgonkivs vgonkivs Awaiting requested review from vgonkivs vgonkivs is a code owner

Assignees

@walldiss walldiss

Labels
area:node Node kind:fix Attached to bug-fixing PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

schema not working as intended in 0.10.4
3 participants
@walldiss @Wondertan @distractedm1nd