Skip to content

Replace olekukonko/tablewriter with text/tabwriter#167

Merged
rst0git merged 1 commit into
checkpoint-restore:mainfrom
adrianreber:2025-08-01-tablewriter
Aug 8, 2025
Merged

Replace olekukonko/tablewriter with text/tabwriter#167
rst0git merged 1 commit into
checkpoint-restore:mainfrom
adrianreber:2025-08-01-tablewriter

Conversation

@adrianreber
Copy link
Copy Markdown
Member

@adrianreber adrianreber commented Aug 1, 2025

After some anonymous third party review, I completed changed the approach to this PR. Instead of using the external dependency tablewriter this now uses text/tabwriter. This reduces included code.

Drop external tablewriter dependency in favor of Go's built-in text/tabwriter package to reduce dependencies and simplify maintenance.

Changes:

  • Replace tablewriter.NewWriter() with tabwriter.NewWriter() in all table display functions
  • Update table formatting logic to use tab-separated output with headers and separator lines
  • Remove olekukonko/tablewriter and related dependencies from go.mod
  • Update test expectations to match new table output format
  • Fix test line number references after table format changes

All tests pass with the new implementation.

Assisted-by: Claude AI for dependency replacement and test updates

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 1, 2025

Test Results

61 tests  ±0   61 ✅ ±0   2s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit e59c4c6. ± Comparison against base commit 6e6a8c6.

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 1, 2025

Codecov Report

❌ Patch coverage is 97.01493% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.01%. Comparing base (6e6a8c6) to head (e7beaf5).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/memparse.go 92.30% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
+ Coverage   71.43%   72.01%   +0.57%     
==========================================
  Files          13       13              
  Lines        1565     1597      +32     
==========================================
+ Hits         1118     1150      +32     
  Misses        373      373              
  Partials       74       74              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@adrianreber adrianreber force-pushed the 2025-08-01-tablewriter branch 3 times, most recently from e63963d to a36cdbc Compare August 1, 2025 15:13
@adrianreber
Copy link
Copy Markdown
Member Author

@rst0git @snprajwal This update was created with Claude. The main reason to create was that dependabot now combines all changes and the tablewriter change breaks API. That is why I wanted to do it in a separate PR.

The new tablewriter comes with new features and new dependencies which increase the binary size. Not sure if this is too much. If I am able to correctly read the number it is 700KB larger than before.

Comment thread .gitignore Outdated
Comment thread cmd/memparse_test.go Outdated
Comment thread cmd/list_test.go Outdated
@rst0git
Copy link
Copy Markdown
Member

rst0git commented Aug 1, 2025

@adrianreber AI models sometimes generate code that is incorrect. It compiles and runs but does not do what we want.

@adrianreber adrianreber force-pushed the 2025-08-01-tablewriter branch from a36cdbc to de26713 Compare August 2, 2025 13:12
@adrianreber
Copy link
Copy Markdown
Member Author

Let me also update README.md.

@adrianreber adrianreber force-pushed the 2025-08-01-tablewriter branch from de26713 to a798d1a Compare August 2, 2025 13:24
@adrianreber
Copy link
Copy Markdown
Member Author

Dropping tablewriter decreases binary size by 400K

@adrianreber adrianreber force-pushed the 2025-08-01-tablewriter branch 2 times, most recently from dcef214 to a53dea9 Compare August 2, 2025 13:44
@rst0git
Copy link
Copy Markdown
Member

rst0git commented Aug 3, 2025

@adrianreber Would it be possible to apply the following changes to your patch?

rst0git@782df66

This approach is similar to the one used in:

@rst0git rst0git changed the title update tablewriter dependency from v0.0.5 to v1.0.9 Replace olekukonko/tablewriter with text/tabwriter Aug 4, 2025
@adrianreber adrianreber force-pushed the 2025-08-01-tablewriter branch from a53dea9 to e59c4c6 Compare August 5, 2025 18:03
@adrianreber
Copy link
Copy Markdown
Member Author

I told claude to update the PR with the changes @rst0git suggested.

Drop external tablewriter dependency in favor of Go's built-in text/tabwriter
package to reduce dependencies and simplify maintenance.

Changes:
- Replace tablewriter.NewWriter() with tabwriter.NewWriter() in all table
  display functions
- Update table formatting logic to use tab-separated output with headers
  and separator lines
- Remove olekukonko/tablewriter and related dependencies from go.mod
- Update test expectations to match new table output format
- Fix test line number references after table format changes

All tests pass with the new implementation.

Assisted-by: Claude AI for dependency replacement and test updates
Signed-off-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Radostin Stoyanov <rstoyano@redhat.com>
@rst0git rst0git force-pushed the 2025-08-01-tablewriter branch from e59c4c6 to e7beaf5 Compare August 8, 2025 08:08
@rst0git
Copy link
Copy Markdown
Member

rst0git commented Aug 8, 2025

I told claude to update the PR with the changes

It seems like Claude doesn't know that it also needs to update the README file.

@rst0git rst0git merged commit 0d35b8d into checkpoint-restore:main Aug 8, 2025
9 checks passed
@adrianreber adrianreber deleted the 2025-08-01-tablewriter branch August 8, 2025 08:13
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.

3 participants