Skip to content

upgrade tablewriter #4026

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

upgrade tablewriter #4026

wants to merge 2 commits into from
+402 −261

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented Jun 3, 2025

What type of PR is this?

/kind cleanup
/kind feature

What this PR does / why we need it:

  • upgrade tablewriter

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Does this PR introduce a user-facing change?

upgrade tablewriter lib

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority area/release-eng Issues or PRs related to the Release Engineering subproject sig/release Categorizes an issue or PR as relevant to SIG Release. labels Jun 3, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested review from jimangel and jrsapi June 3, 2025 10:42
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 3, 2025
@cpanato
Copy link
Member Author

cpanato commented Jun 3, 2025

/assign @xmudrii @Verolop @puerco
cc @kubernetes/release-engineering

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 4, 2025
cpanato added 2 commits June 5, 2025 10:04
@cpanato
upgrade github.com/olekukonko/tablewriter and other deps
c0e5447 
Signed-off-by: Carlos Panato <[email protected]>
@cpanato
migrate to the new table configs
d130c43 
Signed-off-by: Carlos Panato <[email protected]>

more fixes

Signed-off-by: Carlos Panato <[email protected]>

fix lints

Signed-off-by: Carlos Panato <[email protected]>

more fixes

Signed-off-by: Carlos Panato <[email protected]>

revert upgrade for now
@cpanato cpanato force-pushed the update-tablewriter branch from ed5e43f to d130c43 Compare June 5, 2025 08:05
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 5, 2025
@cpanato
Copy link
Member Author

cpanato commented Jun 5, 2025

/assign @xmudrii @puerco @saschagrunert @ameukam

xmudrii
xmudrii requested changes Jun 5, 2025
View reviewed changes
cmd/schedule-builder/cmd/markdown.go

// Check if the next patch release is in the Previous Patch list, if yes dont read in the output
if !patchReleaseInPreviousList(releaseSchedule.Next.Release, releaseSchedule.PreviousPatches) {
table.Append([]string{strings.TrimSpace(releaseSchedule.Next.Release), strings.TrimSpace(releaseSchedule.Next.CherryPickDeadline), strings.TrimSpace(releaseSchedule.Next.TargetDate), ""})
_ = table.Append([]string{strings.TrimSpace(releaseSchedule.Next.Release), strings.TrimSpace(releaseSchedule.Next.CherryPickDeadline), strings.TrimSpace(releaseSchedule.Next.TargetDate), ""})
Copy link
Member

Choose a reason for hiding this comment

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

Why do we completely ignore the error? We should at least log it so we know that there's something wrong going on, otherwise we might just not notice that we're missing some data.

Copy link
Member

Choose a reason for hiding this comment

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

Same in other places where this is present

Copy link
Member Author

Choose a reason for hiding this comment

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

it was already ignoring the error, but will add a log on that

pkg/gcp/build/build.go
@@ -404,14 +406,27 @@ func ListJobs(project string, lastJobs int64) error {
return fmt.Errorf("failed to listing the builds: %w", err)
}

table := tablewriter.NewWriter(os.Stdout)
table.SetHeader([]string{"Start Time", "Finish Time", "Status", "Console Logs"})
table := tablewriter.NewTable(os.Stdout,
Copy link
Member

Choose a reason for hiding this comment

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

Can we add some function in release-utils that's going to return a table that's already configured with everything, so that we don't have to write this much code every time we want to use a table? Does that make sense at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

sg

@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xmudrii xmudrii xmudrii requested changes

@jimangel jimangel Awaiting requested review from jimangel

@jrsapi jrsapi Awaiting requested review from jrsapi

Assignees

@saschagrunert saschagrunert

@Verolop Verolop

@ameukam ameukam

@puerco puerco

@xmudrii xmudrii

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/release Categorizes an issue or PR as relevant to SIG Release. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@cpanato @k8s-ci-robot @xmudrii @saschagrunert @Verolop @ameukam @puerco