Skip to content

Conversation

@JackStouffer
Copy link
Contributor

@JackStouffer JackStouffer commented Feb 2, 2017

These functions are exactly the same as std.utf.encode. And they are quite old, originating in 2007 according to git blame. So they probably existed before std.utf.encode.

They also return a slice over a static array, which is a Bad IdeaTM.

Doing a deprecation cycle because they are publicly accessible and were used in other parts of Phobos.

reserve(size);
foreach (c; ubuf)
buf.ptr[pos++] = c;
}
Copy link
Contributor

@wilzbach wilzbach Feb 12, 2017

Choose a reason for hiding this comment

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

according to CodeCov this isn't used/covered :/

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

This looks quite reasonable to me. However as this is a name deprecation, maybe we should have a second LGTM?

Copy link
Member

@DmitryOlshansky DmitryOlshansky left a comment

Choose a reason for hiding this comment

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

LGTM let's kill the old junk

@DmitryOlshansky
Copy link
Member

Auto-merge toggled on

@DmitryOlshansky DmitryOlshansky merged commit f6e18bf into dlang:master Feb 13, 2017
@WalterBright
Copy link
Member

This introduced a regression: https://issues.dlang.org/show_bug.cgi?id=17227

@wilzbach
Copy link
Contributor

wilzbach commented Feb 26, 2017

This introduced a regression: https://issues.dlang.org/show_bug.cgi?id=17227

I am not sure why you consider deprecation messages as a regression. In particular if

@JackStouffer JackStouffer deleted the toUTF-deprecation branch February 26, 2017 08:25
@CyberShadow
Copy link
Member

have been fixed for the non-Windows part

@wilzbach Why haven't they been fixed for the Windows code in Phobos? I thought fixing all usage of deprecated symbols in Phobos was a hard prerequisite to deprecating anything.

@wilzbach
Copy link
Contributor

wilzbach commented Jul 5, 2017

I thought fixing all usage of deprecated symbols in Phobos was a hard prerequisite to deprecating anything.

AFAICT this has never been required - otherwise the testsuite would run with -de, right?

@CyberShadow
Copy link
Member

Yeah but it doesn't make sense otherwise, does it? Seems antithetical to deprecate something for everyone else but not ourselves.

@CyberShadow
Copy link
Member

Any reason not to enable -de on the test suite then?

@wilzbach
Copy link
Contributor

wilzbach commented Jul 5, 2017

Any reason not to enable -de on the test suite then?

Let's have a look: #5546

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.

5 participants