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

Fix validateEmail/isEmail inconsistency #1582

Merged
merged 3 commits into from Oct 2, 2016

Conversation

Projects
None yet
2 participants
@radcapricorn
Contributor

radcapricorn commented Oct 1, 2016

Proposed fix for issue 1580.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Oct 2, 2016

Member

Interesting, so do I understand it right that isEmail has actually never worked at all? In that case, the affected if statement should be wrapped inside static if (__VERSION__ >= 2072), so that the validation function does at least something useful for the released compiler versions.

Member

s-ludwig commented Oct 2, 2016

Interesting, so do I understand it right that isEmail has actually never worked at all? In that case, the affected if statement should be wrapped inside static if (__VERSION__ >= 2072), so that the validation function does at least something useful for the released compiler versions.

@radcapricorn

This comment has been minimized.

Show comment
Hide comment
@radcapricorn

radcapricorn Oct 2, 2016

Contributor

I'm not sure about "never", perhaps it's just not that widely used so it went unnoticed for a long time. I've added the version check.

Contributor

radcapricorn commented Oct 2, 2016

I'm not sure about "never", perhaps it's just not that widely used so it went unnoticed for a long time. I've added the version check.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Oct 2, 2016

Member

Okay, with "never" I meant something like at least since 2.064 or so, or since vibe.d started to use it.

I've moved the static if to outside of the if*, so that the whole isEmail check is skipped on older DMD versions (doesn't make sense anyway). The basic checks are still performed, so it isn't that bad. The 2.072.0-b1 build presumably will still fail, but I'll fix that after merging this.

* Just used the new PR edit feature for the first time. Definitely a lot nicer/more efficient than requesting the PR author to change small bits here and there.

Member

s-ludwig commented Oct 2, 2016

Okay, with "never" I meant something like at least since 2.064 or so, or since vibe.d started to use it.

I've moved the static if to outside of the if*, so that the whole isEmail check is skipped on older DMD versions (doesn't make sense anyway). The basic checks are still performed, so it isn't that bad. The 2.072.0-b1 build presumably will still fail, but I'll fix that after merging this.

* Just used the new PR edit feature for the first time. Definitely a lot nicer/more efficient than requesting the PR author to change small bits here and there.

@s-ludwig s-ludwig merged commit 16f0430 into vibe-d:master Oct 2, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
@radcapricorn

This comment has been minimized.

Show comment
Hide comment
@radcapricorn

radcapricorn Oct 3, 2016

Contributor

Thanks!

Contributor

radcapricorn commented Oct 3, 2016

Thanks!

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