-
Notifications
You must be signed in to change notification settings - Fork 198
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
Replace isipv6 method with builtin package #1689
Replace isipv6 method with builtin package #1689
Conversation
Dear Vaibhav, Looks nice. Good work. I added some review comments to the code. Beside my comments it would be nice if you could improve tests. The behavior of the function Let me know if I can be of assistance. Best, |
Hey Christian, I am unable to see the review comments. |
Dear Vaibhav,
On 2024-04-15 09:19 Vaibhav Raj ***@***.***> wrote:
Hey Christian, I am unable to see the review comments.
I will get started on the test cases.
I wonder how this could be. Do you read via email or do you visit
github.com? When you visit github.com and visit your on PR you will see
something like this (see attached screenshot). I added two circles with
numbers to the picture to point you to the comments.
Can't you see this on the real github.com website?
Kind
Christian
|
Dear Vaibhav, |
Dear Vaibhav, |
Hey Christian, |
Dear Vaibhav,
That is totally fine for me. Take your time. No need to hurry. I just need to know your schedule to "integrate" your PR into the rest of our workflow and tasks. Sometimes we have contributors throwing in a PR but not finalizing it. Best, |
Dear Christian, Vaibhav |
Dear Vaibhav,
|
Thanks Christian |
Closes #1686
isIPv6Address()
function and related testsipaddress
package directly in theescapeIPv6Address
function