search

taoensso / timbre

Pure Clojure/Script logging library
https://www.taoensso.com/timbre
Eclipse Public License 1.0
1.44k stars 171 forks source link

Remove `println` on core ns load #370

Closed helins closed 1 year ago

helins commented 1 year ago

Libraries printing information without control are often a nuisance. Instead of printing the source of *config*, it is kept in the config itself.

ptaoussanis commented 1 year ago

@AdamClements Hi Adam, thanks for this! Merged, will be included in next release 👍

timgilbert commented 1 year ago

Any chance of getting this released onto clojars? The latest release, 6.1.0, doesn't seem to have it.

ptaoussanis commented 1 year ago

@timgilbert Hi Tim! I do batched work on my libraries, and cut new releases either when there's an important fix or when there's a significant number of changes pending release.

The high-level plan can be found here.

Since Timbre doesn't currently have too many changes pending, it'll probably be at least a few more months before the next release.

Your options in the meantime:

Hope that helps! Cheers :-)

timgilbert commented 1 year ago

Awesome, thanks @ptaoussanis!

finalfantasia commented 1 year ago

@ptaoussanis, these unexpected outputs broke our CI/CD pipeline right after an upgrade today (I know, it's finicky and brittle... but priorities and all that...) and it took us a while to realize it was Timbre. It looks like it's been a few months since the last release in Feb, 2023 and there are a few commits pending release. This one looks pretty low risk to me and I'm wondering if it is possible to have a point release (6.1.1?) with it?

As always, we are grateful for and appreciate all your work on Timbre!

Cheers!

ptaoussanis commented 1 year ago

@finalfantasia Hi Salam,

As always, we are grateful for and appreciate all your work on Timbre!

You're very welcome - I appreciate the kind words.

A couple quick questions:

  1. Just checking that you're aware that [com.taoensso/timbre "6.2.0-alpha1"] is out and includes the change? Is that no good in your case? I understand some places may have policies against pre-release versions. If so, just let me know and I'll try cut a stable release this weekend.

  2. Since you mentioned running into this problem "after an upgrade" - I just want to double check what you mean. A Timbre upgrade, or something else with your system? If a Timbre upgrade, what version were you upgrading from? IIRC, the println output was present in Timbre for quite some time - going back at least a few major versions I believe. Just pointing this out in case there's another issue at work.

Cheers :-)

finalfantasia commented 1 year ago

Just checking that you're aware that [com.taoensso/timbre "6.2.0-alpha1"] is out and includes the change? Is that no good in your case? I understand some places may have policies against pre-release versions. If so, just let me know and I'll try cut a stable release this weekend.

Yes, I was aware of it when I left my last comment. I think I was being too cautious of using an alpha release in the production environment but this is just my personal stance in general that's not driven by any company policies (that I'm aware of). In order to fix the immediate issue with our CI/CD workflow, we ended up using the alpha version after pair-reviewing all the 6 commits (mostly documentation changes) since the last release of 6.1.0, taking into account the rock solid track record of Timbre, and then determining that there's little to no risk in doing so.

Since you mentioned running into this problem "after an upgrade" - I just want to double check what you mean. A Timbre upgrade, or something else with your system? If a Timbre upgrade, what version were you upgrading from?

I was able to isolate and verify the root cause of the issue to the Timbre upgrade from version 5.2.1 to version 6.1.0.

Again, thanks for your contribution to the Clojure community as well as the greater open-source community.

Cheers!

ptaoussanis commented 1 year ago

@finalfantasia Hi Salam,

I think I was being too cautious of using an alpha release in the production environment but this is just my personal stance

That's very reasonable, I'd encourage you to continue with that perspective. In this case it just happened to be that that particular alpha is equivalent to the stable point release I would have otherwise pushed.

I was able to isolate and verify the root cause of the issue to the Timbre upgrade from version 5.2.1 to version 6.1.0.

Okay, great 👍 And indeed I was mistaken- the problematic println was only added in v6, I thought it had been there for much longer. Apologies for the trouble that caused!

Again, thanks for your contribution to the Clojure community as well as the greater open-source community.

You're very welcome, thanks for saying so!

Cheers :-)

ptaoussanis commented 1 year ago

Looks like this is causing trouble for Sente. Re-opening until there's a stable release that includes the merged fix.

Will get one out before end of June 👍

ptaoussanis commented 1 year ago

Pushing a new stable release of Timbre shortly, marking as done.