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

rm tests of deprecated logging functions #25229

Closed
wants to merge 1 commit into from

Conversation

stevengj
Copy link
Member

As @StefanKarpinski suggested (#25067 (comment)), this PR deletes the tests of the logging functions that were deprecated in #24490.

In fact, these tests had apparently already bitrotted because they aren't exercised by CI (which runs with --depwarn=error, I guess), making make -C tests misc break if you run it locally.

@stevengj stevengj added deprecation This change introduces or involves a deprecation test This change adds or pertains to unit tests labels Dec 21, 2017
@stevengj stevengj requested a review from c42f December 21, 2017 17:41
@stevengj
Copy link
Member Author

MacOS Travis failure No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself. seems unrelated.

Copy link
Member

@c42f c42f 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 fine to me.

I had an alternative for this which I thought addressed your concerns at 5d77d1b#r26334167 a little more thoroughly.

It was coming as part of a larger cleanup of small odds and ends, but here it is #25239

Either way seems reasonable to me, though I think testing deprecations as a matter of course would be good practice. It should be as simple as moving them to a section in deprecation_exec.jl just like we do for base/deprecated.jl

@c42f
Copy link
Member

c42f commented Dec 22, 2017

Hopefully I manage to remove the need for --depwarn=error soon so we won't need to run deprecation_exec.jl with special flags.

This was referenced Dec 22, 2017
@c42f
Copy link
Member

c42f commented Dec 22, 2017

Superseded by #25239

@c42f c42f closed this Dec 22, 2017
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 test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants