search

invenia / Memento.jl

A flexible logging library for Julia
https://invenia.github.io/Memento.jl/latest
Other
87 stars 14 forks source link

Memento.Test might need to be renamed #111

Closed ararslan closed 5 years ago

ararslan commented 5 years ago

Since the Test stdlib module is no longer namespaced within Base, the Test identifier brought into scope upon using Test conflicts with that from using Memento.Test, but in a weird, non-fatal way that somehow still seems to work.

As an example, the following warning is emitted when running Memento's own tests:

WARNING: using Test.Test in module Main conflicts with an existing identifier.

We also get this warning when loading an internal package that depends on Memento, but I think this is just my fault from #109, inadvertently omitting Test from the Project.toml:

┌ Warning: Package Memento does not have Test in its dependencies:
│ - If you have Memento checked out for development and have
│   added Test as a dependency but haven't updated your primary
│   environment's manifest file, try `Pkg.resolve()`.
│ - Otherwise you may need to report an issue with Memento
└ Loading Test into Memento from project dependency, future warnings for Memento are suppressed.

I would propose that Memento.Test be renamed to Memento.TestUtils or similar.

rofinn commented 5 years ago

I'm inclined to just remove the Test module then. This seems like an annoying property of julia namespacing, since we only care about the exports and overloaded macros.

ararslan commented 5 years ago

Do you mean remove the functionality or just move it out of the submodule and into the main Memento module?

rofinn commented 5 years ago

Move it out of the submodule and into the main Memento module. FWIW, submodules in julia just seem like more of a hassle than they're worth... which is pretty disappointing. I mostly blame using.

iamed2 commented 5 years ago

The only problem seems to me to be that you can't import everything in Memento.Test without importing the module Memento.Test as Test.

rofinn commented 5 years ago

True, I just find that pretty annoying.