search

checkpoint-restore / checkpointctl

A tool for in-depth analysis of container checkpoints
Apache License 2.0
87 stars 15 forks source link

Refactor: Split commands into separate package #117

Closed shikharish closed 5 months ago

shikharish commented 5 months ago

Closes #80.

codecov-commenter commented 5 months ago

Codecov Report

Attention: 102 lines in your changes are missing coverage. Please review.

Comparison is base (9aa0386) 78.85% compared to head (fd1ee62) 78.67%. Report is 11 commits behind head on main.

Files Patch % Lines
internal/json.go 66.66% 58 Missing and 20 partials :warning:
internal/utils.go 66.00% 12 Missing and 5 partials :warning:
cmd/memparse.go 90.41% 4 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #117 +/- ## ========================================== - Coverage 78.85% 78.67% -0.19% ========================================== Files 6 9 +3 Lines 927 1158 +231 ========================================== + Hits 731 911 +180 - Misses 154 185 +31 - Partials 42 62 +20 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 5 months ago

Test Results

49 tests  ±0   49 :white_check_mark: ±0   1s :stopwatch: ±0s  1 suites ±0    0 :zzz: ±0   1 files   ±0    0 :x: ±0 

Results for commit fd1ee62a. ± Comparison against base commit 311918ee.

:recycle: This comment has been updated with latest results.

shikharish commented 5 months ago

@snprajwal @adrianreber Any reviews?

snprajwal commented 5 months ago

@shikharish can you please squash all the commits into a single one? The first commit not being signed-off triggers the DCO failure. It would also be helpful to rename the PR to something like refactor: split commands into separate package

shikharish commented 5 months ago

Done. @snprajwal

shikharish commented 5 months ago

@snprajwal Any more reviews?

snprajwal commented 5 months ago

@rst0git @adrianreber PTAL

snprajwal commented 5 months ago

Also, @shikharish you would have to rebase this branch against the main branch of the repo, some commits have been added in the past week

rst0git commented 5 months ago

@shikharish Would you be able to add a git commit message describing what changes have been made? It would make it much easier to review ;-)

shikharish commented 5 months ago

@rst0git I have squashed all commits into one. I believe you can see that one with all commit messages. Should I make a more detailed commit message?

rst0git commented 5 months ago

Should I make a more detailed commit message?

@shikharish It is a good practice to do that. The following contributing guidelines and blog post provide more information:

shikharish commented 5 months ago

@rst0git Updated commit message. I hope this is better. Thank you for the informative resources.

adrianreber commented 5 months ago

Looks good indeed. All tests look happy.