Skip to content

Comments

Print receive ping moved to new functions#400

Merged
gsnw-sebast merged 1 commit intoschweikert:developfrom
gsnw:print_recv
Aug 25, 2025
Merged

Print receive ping moved to new functions#400
gsnw-sebast merged 1 commit intoschweikert:developfrom
gsnw:print_recv

Conversation

@gsnw-sebast
Copy link
Collaborator

I have split the output below the function int wait_for_reply(int64_t wait_time) into four new functions.

  • print_recv
  • print_recv_json
  • print_recv_ext
  • print_recv_ext_json

This should make it easier to read.

@coveralls
Copy link

coveralls commented Jul 28, 2025

Coverage Status

coverage: 86.511% (+0.3%) from 86.235%
when pulling 613a5b4 on gsnw:print_recv
into a54fb46 on schweikert:develop.

@gsnw-sebast gsnw-sebast requested a review from auerswal August 2, 2025 05:24
Copy link
Collaborator

@auerswal auerswal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if you could move the computations outside of the print functions. They are currently duplicated, and just moving the code into additional functions keeps them duplicated. Ideally, moving printing to additional functions should reduce code duplication.

@gsnw-sebast gsnw-sebast requested a review from auerswal August 19, 2025 05:45
@gsnw-sebast gsnw-sebast force-pushed the print_recv branch 3 times, most recently from 7bdc64b to 959c7ed Compare August 22, 2025 06:32
@gsnw-sebast gsnw-sebast requested a review from schweikert August 22, 2025 14:06
@gsnw-sebast gsnw-sebast changed the title Print recive ping moved to new functions Print receive ping moved to new functions Aug 23, 2025
@gsnw-sebast gsnw-sebast merged commit 2fd76cd into schweikert:develop Aug 25, 2025
9 checks passed
@gsnw-sebast gsnw-sebast deleted the print_recv branch August 26, 2025 11:46
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.

4 participants