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 deprecation for equalto and occursin #26480

Merged
merged 1 commit into from
Mar 19, 2018
Merged

Conversation

ararslan
Copy link
Member

@ararslan ararslan commented Mar 15, 2018

These were removed in #26436 without deprecation, so this PR adds a deprecation for them plus a NEWS item. It also addresses #26436 (comment).

@ararslan ararslan added the deprecation This change introduces or involves a deprecation label Mar 15, 2018
@mbauman
Copy link
Member

mbauman commented Mar 15, 2018

I'd remove the NEWS — the context there is breaking changes from 0.6. I think that'd just be confusing for the vast majority of the audience.

@ararslan ararslan force-pushed the aa/equalto-dep branch 2 times, most recently from 84b6fcf to e8f954c Compare March 15, 2018 23:13
@@ -1510,6 +1510,12 @@ end
# Issue #26248
@deprecate conj(x) x

# PR #26436
@deprecate equalto(x) isequal(x)
@deprecate(occursin(x), in(x))
Copy link
Member

Choose a reason for hiding this comment

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

Why the parens?

Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise it parses as occursin(x) in x

Copy link
Member

Choose a reason for hiding this comment

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

d'oh!

@deprecate equalto(x) isequal(x)
@deprecate(occursin(x), in(x))
@deprecate_binding EqualTo Base.Fix2 false
@deprecate_binding OccursIn Base.Fix2 false
Copy link
Member

Choose a reason for hiding this comment

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

I left in definitions for these actually:

const EqualTo = Fix2{typeof(isequal)}
const OccursIn = Fix2{typeof(in)}

I'm not sure what their ultimate fate will be but they're pretty harmless for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

The use of OccursIn there will be kind of weird once the contains -> occursin replacement goes through though.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, but we have to do something. They can't both be deprecated just to Fix2, since that won't work.

Copy link
Member Author

@ararslan ararslan Mar 16, 2018

Choose a reason for hiding this comment

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

Okay, I can just remove these deprecations and we can probably just silently remove EqualTo and OccursIn between 0.7 and 1.0. How does that sound?

Copy link
Member

Choose a reason for hiding this comment

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

Why not just do @deprecate_binding EqualTo Base.Fix2{typeof(isequal)} false? That seems to work.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we want to make the internal definitions use <:Union{typeof(isequal), typeof(==)}?

Also deprecate the unexported EqualTo and OccursIn aliases (which were
previously types before 26436).
@JeffBezanson JeffBezanson merged commit 9a55c8f into master Mar 19, 2018
@JeffBezanson JeffBezanson deleted the aa/equalto-dep branch March 19, 2018 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants