Logging.jl icon indicating copy to clipboard operation
Logging.jl copied to clipboard

RFC: Unexport logging functions

Open kmsquire opened this issue 9 years ago • 4 comments

  • Two of the logging functions (info and warn) conflict with functions in base, and error was originally named err for the same reason. Unexporting them removes these conflicts.

  • With this change, the functions will (eventually) have to be qualified.

    using Logging
    Logging.@configure(level=DEBUG)
    
    Logging.debug("Debug message")
    Logging.info("Information for you!")
    Logging.warn("This is a warning.")
    Logging.error("You did something wrong!")
    Logging.critical("Danger Will Robinson!")
    

    A user can still call these directly without qualification (and without override warnings) with

    using Logging
    import Logging: debug, info, warn, error, critical
    Logging.@configure(level=DEBUG)
    
    debug("Debug message")
    info("Information for you!")
    warn("This is a warning.")
    error("You did something wrong!")
    critical("Danger Will Robinson!")
    
  • Also deprecate Logging.err in favor of Logging.error. This wasn't possible before because of conflicts with Base.error

  • In order to properly deprecate the non-qualified version, the Logging.configure function was also removed in favor of Logging.@configure, and the unqualified names are imported (with a deprecation warning) when the user calls Logging.@configure, but only if 1) Logging was included with using Logging, and 2) the user hasn't already imported the names manually. (This required a few shenanigans, and might be brittle, but should work most of the time and should only be temporary--e.g., until Julia v0.6).

Looking for comments. My plan would be to release this as v0.5 or later.

Cc: @sbchisholm @aviks @DanielArndt @joshday @lostanlen @JoshChristie @fiatflux @zxteloiv

kmsquire avatar Oct 10 '16 17:10 kmsquire

Thanks @JoshChristie. I think I found all of the needed renames.

Regarding keeping Logging.configure() vs solely relying on Logging.@configure(), that depends on how fast we want to break things.

Until now, the recommended way to use Logging was to call using Logging, and then use the logging functions directly. I would guess that most code does this, and and I would prefer a way to warn users with a deprecation, rather than break their code.

The way I do that here is by importing deprecated versions of the functions in the Logging.@configure macro call, with instructions on how to fix the offending code. This isn't possible from a function, and without that import, all code which calls the logging functions directly would fail.

Importing the functions in this way is a little sketchy--e.g., it doesn't work if you call Logging.@configure from a function, although it does fail gracefully, with a warning.

An alternative would be to issue the deprecation warning, and then follow through in a month or three (or, e.g., when Julia v0.6 comes out). That might be fine (and the resulting code would be simpler), although we would have to live with the current behavior during that time (overwriting Base.info and Base.warn).

I'll try to get that PR out for comparison, and then we can make a decision.

kmsquire avatar Oct 11 '16 00:10 kmsquire

Thanks for the explanation. That makes sense.

However, when i tried this out i get an error. Should this be a warning instead?

julia> using Logging
julia> Logging.configure(level=INFO)
ERROR: The functional form of Logging.configure(...) is no longer supported.
Instead, call
    Logging.@configure(...)
 in (::Logging.#kw##configure)(::Array{Any,1}, ::Logging.#configure) at ./<missing>:0

One last thing, renaming in the README.md should be Logging.@configure().

JoshChristie avatar Oct 11 '16 01:10 JoshChristie

Yes, making it a warning would be better--I'll do that.

kmsquire avatar Oct 11 '16 02:10 kmsquire

What is the status of this PR?

lostanlen avatar Mar 08 '17 23:03 lostanlen