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

add bracket on node if using proxy and node has ipv6 address #2245

Merged
merged 5 commits into from
Feb 26, 2021
Merged

add bracket on node if using proxy and node has ipv6 address #2245

merged 5 commits into from
Feb 26, 2021

Conversation

davama
Copy link
Contributor

@davama davama commented Jan 8, 2021

Should solve: #2228

Note:
Not a ruby dev so i am sure there is a better way to do it.
Open to suggestions.

I also tested if node has an ipv4 address and it also works. So basically, at least from what i can tell, it does not break anything. 😆

Thanks,
Dave

opted to use "include" to validate if the ip address has the colon `:` string, instead of `ipaddress` gem
looks like `[ip]` works even with ipv4 address.
so no need for an if statement.
much simpler change
@codecov-io
Copy link

codecov-io commented Jan 11, 2021

Codecov Report

Merging #2245 (91517d7) into master (c0dae48) will decrease coverage by 0.71%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2245      +/-   ##
==========================================
- Coverage   63.30%   62.59%   -0.72%     
==========================================
  Files          30       30              
  Lines        1496     1497       +1     
==========================================
- Hits          947      937      -10     
- Misses        549      560      +11     
Impacted Files Coverage Δ
lib/oxidized/input/ssh.rb 45.45% <100.00%> (-11.12%) ⬇️
lib/oxidized/node.rb 70.83% <0.00%> (+0.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0dae48...91517d7. Read the comment docs.

@davama
Copy link
Contributor Author

davama commented Jan 11, 2021

find it strange that Travis complains when the change is soo minor....

I think other projects on github were having travis issues recently....

@davama
Copy link
Contributor Author

davama commented Feb 26, 2021

Sorry for the bump...

Just wanted to bring to the attention, since this change is literally the addition of two characters 😄
There is another proxy related PR which are very interesting. Hopefully it's merged soon too

Happy Friday!!

@ytti
Copy link
Owner

ytti commented Feb 26, 2021

Apologies and thank you.

@ytti ytti merged commit 8179f48 into ytti:master Feb 26, 2021
@davama
Copy link
Contributor Author

davama commented Feb 26, 2021

@ytti
No apologies needed here!
Thank you very much!!

have a good weekend

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