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.

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

ZmqLogger: Add optional dumping to stderr #422

Merged
merged 5 commits into from Oct 29, 2020

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Feb 1, 2020

Add an openshot::Settings boolean DEBUG_TO_STDERR, which when set true will direct all AppendDebugMethod() messages to std::clog, in addition to (possibly) being sent over the ZeroMQ wire.

Set to WIP because I want to add unit tests and check it over a bit more, right now this is a quick and dirty proof-of-concept implementation intended for #420.

Also:

  • Replaced usleep() call with the C++11 std::this_thread::sleep_for()
  • Added std:: prefixes throughout ZmqLogger.cpp, and removed the using namespace std;
  • Corrected some cut-and-paste comment errors in Settings.h
  • Reformatted a bunch of code for readability / differing tab widths

Add an openshot::Settings boolean `DEBUG_TO_STDERR`, which when
set true will direct all AppendDebugMethod() messages to std::clog,
in addition to (possibly) being sent over the ZeroMQ wire.
@codecov-io
Copy link

codecov-io commented Feb 1, 2020

Codecov Report

Merging #422 into develop will decrease coverage by 0.00%.
The diff coverage is 36.36%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #422      +/-   ##
===========================================
- Coverage    49.45%   49.44%   -0.01%     
===========================================
  Files          129      129              
  Lines        10261    10265       +4     
===========================================
+ Hits          5075     5076       +1     
- Misses        5186     5189       +3     
Impacted Files Coverage Δ
src/Settings.h 50.00% <ø> (ø)
src/ZmqLogger.cpp 28.20% <22.22%> (-1.13%) ⬇️
src/Settings.cpp 100.00% <100.00%> (ø)
src/ZmqLogger.h 33.33% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 07a447c...f55c563. Read the comment docs.

@jonoomph
Copy link
Member

@ferdnyc Is this PR ready for merge? It seems complete to me, but it still has the WIP decorator. If it's ready, slam that merge button! 👍 Thx.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Oct 17, 2020

@jonoomph

I think it is ready. I had originally wanted to add unit tests, but I've since reconsidered given the issues I've already encountered trying to unit-test singletons. (Plus, I think we'd need a more full-stack unit test framework than just UnitTest++, to test output to stderr in any meaningful way.)

I'll see what state it's in and finish up whatever's needed, it would be good to have. I also wanted to integrate the functionality into OpenShot somehow, and with --debug support merged things are in a much more conducive state over on that side.

@jonoomph
Copy link
Member

Thanks, LGTM!

@jonoomph jonoomph changed the title WIP: ZmqLogger: Add optional dumping to stderr ZmqLogger: Add optional dumping to stderr Oct 29, 2020
@jonoomph jonoomph merged commit 896b307 into OpenShot:develop Oct 29, 2020
@ferdnyc ferdnyc deleted the debug-to-stderr branch November 3, 2020 13:20
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.

None yet

3 participants