hides debug/info/warn messages behind a flag #633
Conversation
|
|
||
| def info(msg: String): Unit = | ||
| c.info(c.enclosingPosition, msg, force = true) | ||
| if (debugEnabled) c.info(c.enclosingPosition, msg, force = true) | ||
|
|
||
| def debug[T](v: T): T = { |
There was a problem hiding this comment.
this method is used only for the development, please remove the condition from it.
There was a problem hiding this comment.
@fwbrasil Where should it otherwise be? In debug()?
| @@ -16,13 +21,13 @@ object Messages { | |||
| c.abort(c.enclosingPosition, msg) | |||
|
|
|||
| def warn(msg: String): Unit = | |||
| c.warning(c.enclosingPosition, msg) | |||
| if (debugEnabled) c.warning(c.enclosingPosition, msg) | |||
There was a problem hiding this comment.
I think we should keep the warnings always enabled.
| @@ -4,6 +4,11 @@ import scala.reflect.macros.blackbox.{ Context => MacroContext } | |||
|
|
|||
| object Messages { | |||
|
|
|||
| private val debugEnabled = | |||
There was a problem hiding this comment.
It seems that this will disable the logs by default, is it intentional? I'd prefer to keep them enabled by default.
There was a problem hiding this comment.
yes this is intentional, I mean our CI system or a clean compile produces over >10k lines on our systems (already).
There was a problem hiding this comment.
I mean the problem is that it's really not usable as soon as you hit a certain amount of queries, also quill still produces runtime logs for the query, which is more than enough. (this is only compile time removal)
Edit: and it's also possible to use the SqlMirrorContext.
There was a problem hiding this comment.
@schmitch This would disable by default one of the most important features that Quill was designed to provide: compile time feedback of the generated query. I still think it should be enabled by default. @getquill/maintainers wdyt?
There was a problem hiding this comment.
I'd prefer to keep old behavior by default and add instructions about disabling verbose compilation log to README.md. It should have defaults friendly to beginners and later everyone can change settings to their needs. Compile-time query generation is the killer feature that should be demonstrated right after developers setup Quill in their projects.
There was a problem hiding this comment.
I actually updated the PR and added a ENV variable. Also I flipped the switch, so that logging is now enabled by default, but could be enabled by either setting env or a system property.
Well the best way would be having a way to set the Log Level, but at the moment I don't know any possibility to do so with compile time macro's at least you could disable them with logLevel := Level.Warn but unfortunatly that disable's also all info logs of the scala compiler, so it would be really really great to have a log level for the quill code only, but at the moment I don't know any "good" way to do it.
--
Maybe that's the better solution. But well I actually think it's definitly not the most important feature. Actually once you stop doing 10 liners it actually gets annoying.
|
@schmitch sorry for the late feedback. The build is failing because there's a source file unformatted: https://travis-ci.org/getquill/quill/builds/180329536#L9968 Could you please fix it? Thanks! |
|
Done ;) |
|
thanks @schmitch! |
Fixes #632
Problem
When you people reach more and more compiled queries the log gets overhauled whenever doing a
sbt clean compilerun. This hides the debug/info/warn output behind a flag, so that whenever somebody wants to enable it, they need to run viasbt -Dquill.macro.log=true. (Does not disable, fail and error, maybe I will exclude warn as well?)More info in Ticket.
P.S.: not sure if this is testable :-(
@getquill/maintainers