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

add udp reachable check #545

Merged
merged 4 commits into from
Feb 17, 2020
Merged

add udp reachable check #545

merged 4 commits into from
Feb 17, 2020

Conversation

weakiwi
Copy link
Contributor

@weakiwi weakiwi commented Feb 11, 2020

add integration test for udp

Checklist
  • make test-all (UNIX) passes. CI will also test this
  • unit and/or integration tests are included (if applicable)

Description of change

add udp reachable check
6e5beac 
add integration test for udp
@weakiwi
Copy link
Contributor Author

weakiwi commented Feb 11, 2020 •
edited
Loading

Related to #542

pedroMMM
pedroMMM approved these changes Feb 12, 2020
View reviewed changes
Copy link
Contributor

@pedroMMM pedroMMM left a comment

Choose a reason for hiding this comment

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

Simple and to the point, great!

The only feedback I have is that you forgot to add something like fix #542 in the PR Description like the documentation that is linked on the PR Template askes.

aelsabbahy
aelsabbahy requested changes Feb 12, 2020
View reviewed changes
Copy link
Member

@aelsabbahy aelsabbahy left a comment

Choose a reason for hiding this comment

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

Looks great, requesting a minor refactor.

Thanks for the contribution.

system/addr.go Show resolved Hide resolved
@aelsabbahy
Copy link
Member

For what it's worth, I think the whole fixes #542 works in the comments too. I'll see if this comment ends up closing it when the ticket is merged

@aelsabbahy
Copy link
Member

aelsabbahy commented Feb 13, 2020 •
edited
Loading

@weakliwi I'll review and merge this sometime tomorrow.

Thank you for your contribution!

@aelsabbahy
Copy link
Member

Sorry for running late on the merge of this.

As long as there are no merge conflicts you don't have to keep merging or rebasing master.

I'll try to get this done tomorrow or first thing Monday.

@weakiwi
Copy link
Contributor Author

weakiwi commented Feb 16, 2020

Sorry for running late on the merge of this.

As long as there are no merge conflicts you don't have to keep merging or rebasing master.

I'll try to get this done tomorrow or first thing Monday.

OK. Get it

@aelsabbahy
Copy link
Member

Looks great, thanks!

@aelsabbahy aelsabbahy merged commit 81d5f15 into goss-org:master Feb 17, 2020
@aelsabbahy
Copy link
Member

@pedroMMM I was wrong, it didn't auto mark #542 as closed. Closing it manually.

BenjaminHerbert pushed a commit to BenjaminHerbert/goss that referenced this pull request May 28, 2020
@weakiwi @BenjaminHerbert
add udp reachable check (goss-org#545)
30325ec 
* add udp reachable check

add integration test for udp

* refactor for how variable creating
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pedroMMM pedroMMM pedroMMM approved these changes

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@weakiwi @aelsabbahy @pedroMMM