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

refactor: move table rendering into own funcs #60

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

snprajwal
Copy link
Member

The logic for rendering tables with the checkpoint data was all present
in a single function showContainerCheckpoint(). This has now been
refactored to provide each table its own function that can be called to
render it. This increases code maintainability going forward as more and
more tables are added to the checkpointctl show command.

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2023

Codecov Report

Patch coverage: 90.14% and project coverage change: +0.25 🎉

Comparison is base (da69a6b) 80.31% compared to head (2d433eb) 80.56%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #60      +/-   ##
==========================================
+ Coverage   80.31%   80.56%   +0.25%     
==========================================
  Files           3        3              
  Lines         386      391       +5     
==========================================
+ Hits          310      315       +5     
  Misses         55       55              
  Partials       21       21              
Impacted Files Coverage Δ
container.go 82.78% <89.23%> (+0.36%) ⬆️
checkpointctl.go 84.21% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link

github-actions bot commented Jun 4, 2023

Test Results

23 tests  ±0   23 ✔️ ±0   0s ⏱️ ±0s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ±0 

Results for commit 2d433eb. ± Comparison against base commit da69a6b.

♻️ This comment has been updated with latest results.

@snprajwal snprajwal force-pushed the refactor branch 2 times, most recently from 590b3c8 to b0fd761 Compare June 8, 2023 17:03
The logic for rendering tables with the checkpoint data was all present
in a single function `showContainerCheckpoint()`. This has now been
refactored to provide each table its own function that can be called to
render it. This increases code maintainability going forward as more and
more tables are added to the `checkpointctl show` command.

Signed-off-by: Prajwal S N <prajwalnadig21@gmail.com>
The flag variables for the CLI were previously using either `print` or
`show` prefixes. This is not required as they are booleans used to
indicate if the flag is enabled. The names have now been made consistent
to enable future flag variables to be named idiomatically.

Signed-off-by: Prajwal S N <prajwalnadig21@gmail.com>
Copy link
Member

@rst0git rst0git left a comment

Choose a reason for hiding this comment

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

LGTM

@rst0git rst0git merged commit fa6a6f2 into checkpoint-restore:main Jun 8, 2023
@snprajwal snprajwal deleted the refactor branch June 8, 2023 18:14
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