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.

Sign up for GitHub

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

Jump to bottom

Suppress dbg! in release build #58383

Closed
jfcarr opened this issue Feb 11, 2019 · 5 comments
Closed

Suppress dbg! in release build #58383

jfcarr opened this issue Feb 11, 2019 · 5 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@jfcarr
Copy link

jfcarr commented Feb 11, 2019

I'm hesitant to open this issue, because I suspect it has an obvious answer that I'm missing.

Anyway...

I love the new dbg! macro in 1.32.0. But, given that its purpose is outputting debugging messages, should its output be suppressed in a project's release build? (cargo build --release)
@sfackler
Copy link
Member

This was discussed in the RFC: rust-lang/rfcs#2361

@scottmcm
Copy link
Member

scottmcm commented Feb 11, 2019 •
edited
Loading

I'm a fan of leaving this as-is -- I used dbg! in one of my "takes 2 minutes to run in release mode, and who knows how long in debug" projects, and was very glad for the output in release.

Maybe it would make sense for the dbg! documentation to point to log::debug! for when people want to leave these lines checked in? I'm not usually a fan of pointing to external crates from std, but since it's in the nursery (https://github.com/rust-lang-nursery/log) maybe it's ok to make an exception?

@Centril
Copy link
Contributor

Centril commented Feb 11, 2019

Maybe it would make sense for the dbg! documentation to point to log::debug! for when people want to leave these lines checked in? I'm not usually a fan of pointing to external crates from std, but since it's in the nursery (https://github.com/rust-lang-nursery/log) maybe it's ok to make an exception?

We already do that, https://doc.rust-lang.org/nightly/std/macro.dbg.html -- but it could use some linkification and elaboration...

@kennytm kennytm added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Feb 12, 2019
@DevQps
Copy link
Contributor

DevQps commented Mar 27, 2019

If there are no people that are willing to pick this up, I don't mind picking this up!
If I understand the conversation correctly the work that needs to be done is basically:

  • Improve the documentation such that it's clear that the dbg! macro will also work in release builds (for testing reasons). But should it should be discouraged to actually use it for logging in (published) release builds.
  • Make it clear that if people want debug statements to work only in debug builds log::debug! is better suited.

If I have a wrong view of this or if anyone else wants to pick this up, please let me know!

@Centril
Copy link
Contributor

Centril commented Mar 27, 2019

  • But should it should be discouraged to actually use it for logging in (published) release builds.

  • Make it clear that if people want debug statements to work only in debug builds log::debug! is better suited.

This is already the case.

Improve the documentation such that it's clear that the dbg! macro will also work in release builds (for testing reasons).

This remains to be done.

@DevQps DevQps mentioned this issue Mar 29, 2019
Closed
Centril added a commit to Centril/rust that referenced this issue Mar 29, 2019
@Centril
Rollup merge of rust-lang#59528 - DevQps:improve-dbg-macro-docs, r=Ce…
abd3db9 
…ntril

Improve the dbg! macro docs

# Description

As stated has been discussed in rust-lang#58383 the docs do not clearly state why it is useful to have the option to use `dbg!` in release builds as well. This PR should change that.

closes rust-lang#58383
@bors bors closed this as completed in 183afcd Mar 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve the dbg! macro docs DevQps/rust
6 participants
@kennytm @Centril @sfackler @jfcarr @scottmcm @DevQps