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

Support IPv6 Urls #1341

Merged
merged 2 commits into from Dec 17, 2015

Conversation

Projects
None yet
2 participants
@Marenz
Contributor

Marenz commented Nov 30, 2015

I tried running the unittests but somehow they all succeeded even though I deliberately broke one.
Still, the code seems to work with a simple test. I am secretly hoping for a CI that tests for me

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 2, 2015

Member

The unit test issue is probably due to the split into sub packages. Need to use either dub test --combined or dub test :core now.

The diff looks good so far. The only thing I wonder is if m_is_ipv6 should be replaced by a check for .canFind(":") in toString(), to save space, but more importantly to support setting the .host property to an IPv6-address after construction.

Member

s-ludwig commented Dec 2, 2015

The unit test issue is probably due to the split into sub packages. Need to use either dub test --combined or dub test :core now.

The diff looks good so far. The only thing I wonder is if m_is_ipv6 should be replaced by a check for .canFind(":") in toString(), to save space, but more importantly to support setting the .host property to an IPv6-address after construction.

@Marenz

This comment has been minimized.

Show comment
Hide comment
@Marenz

Marenz Dec 5, 2015

Contributor

Updated to use canFind and extended unittest to setting .host to a ipv6

Contributor

Marenz commented Dec 5, 2015

Updated to use canFind and extended unittest to setting .host to a ipv6

Show outdated Hide outdated source/vibe/inet/url.d
@Marenz

This comment has been minimized.

Show comment
Hide comment
@Marenz

Marenz Dec 17, 2015

Contributor

Changed return type to auto. Rebased on latest master.

Contributor

Marenz commented Dec 17, 2015

Changed return type to auto. Rebased on latest master.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Dec 17, 2015

Member

Okay, thanks!

Member

s-ludwig commented Dec 17, 2015

Okay, thanks!

Marenz added some commits Nov 30, 2015

Replace spaces with tabs in url.d
This way it's consistent with the rest of the file
@Marenz

This comment has been minimized.

Show comment
Hide comment
@Marenz

Marenz Dec 17, 2015

Contributor

importing std.algorithm : canFind now

Contributor

Marenz commented Dec 17, 2015

importing std.algorithm : canFind now

s-ludwig added a commit that referenced this pull request Dec 17, 2015

@s-ludwig s-ludwig merged commit f753544 into vibe-d:master Dec 17, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Marenz Marenz deleted the Marenz:urlipv6 branch Dec 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment