Skip to content

Use log facade in testbed2d. - #129

Closed
ThierryBerger wants to merge 19 commits into
dimforge:masterfrom
ThierryBerger:master
Closed

Use log facade in testbed2d.#129
ThierryBerger wants to merge 19 commits into
dimforge:masterfrom
ThierryBerger:master

Conversation

@ThierryBerger

Copy link
Copy Markdown
Contributor

To illustrate what I'm suggesting with issue #128

I can make the same update for testbed3d if you're interested.

@sebcrozet

Copy link
Copy Markdown
Member

Thanks, that's a good suggestion!

I can make the same update for testbed3d if you're interested.

Definitely interested!

@ThierryBerger

Copy link
Copy Markdown
Contributor Author

Glad you like it.

Issues to take into account :

  • When running the examples, we lose the logs (as no log implementation is set).

I suggest to choose the most minimal one and add it as a dependency of all examples. simple-logger or env-logger are both good candidates.

I chose env_logger arbitrarily (I like their documentation most)

I will update the pull request with it.

Also, by default, logs are set to a minimum, as per env_logger documentation :

Log levels are controlled on a per-module basis, and by default all logging is disabled except for error!. Logging is controlled via the RUST_LOG environment variable. The value of this environment variable is a comma-separated list of logging directives. A logging directive is of the form:

So to enable them back, you have to invoke the examples like that:
RUST_LOG=nphysics_testbed2d cargo run --bin balls2

Perhaps the nphysics documentation should reflect that.

@sebcrozet

Copy link
Copy Markdown
Member

Thank you for your suggestion here. I am going to close this PR in favor of #143 which:

  • Enables the log facade only if the log feature is enabled.
  • Does not change the examples (which keep using the default behavior based on println).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants