TLM: Upgrade to v1 tablewriter #7937
Conversation
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jh125486
force-pushed
the
update_tablewriter
branch
3 times, most recently
from
f1e1df4 to
1aff7c5
Compare
srenatus
left a comment
srenatus
left a comment
There was a problem hiding this comment.
Thank you! That's quite an effort. I'll let @johanfylling and @sspaink have another look before merging, but I'm happy that this is resolved. 👏
|
Would you mind rebasing this? Github reports conflicts, probably due to go.sum/go.mod changes. |
|
This is great. Thank you, Jacob! @jh125486 👍 |
There was a problem hiding this comment.
Thank you for looking into this 😃. Since this wasn't on a perceived critical path, it would likely have taken some time before we got around to it. Much appreciated!
Changes LGTM 👍. Just some tests that don't report failures anymore and need amending.
| t.SetAutoMergeCellsByColumnIndex([]int{0}) | ||
| t = t.Options(tablewriter.WithConfig(tablewriter.NewConfigBuilder(). | ||
| ForColumn(0).Build().Row().Formatting().WithMergeMode(tw.MergeVertical). // vertical merge column 0. | ||
| Build().Build().Build(), // build the table config. |
There was a problem hiding this comment.
Go, go, go! (sorry, couldn't help it 😄)
- Updated test fixtures and various test fixes. Signed-off-by: Jacob Hochstetler <[email protected]>
jh125486
force-pushed
the
update_tablewriter
branch
from
5d549f8 to
d3ddf2a
Compare
Signed-off-by: Jacob Hochstetler <[email protected]>
jh125486
force-pushed
the
update_tablewriter
branch
from
e16ca29 to
cae67ed
Compare
Signed-off-by: Jacob Hochstetler <[email protected]>
Signed-off-by: Jacob Hochstetler <[email protected]>
|
The out of sync branch was weird... when I updated my branch last night I think GH hadn't cached the latest :/ I rebased, bu then missed a commit on rebase, so it took me a bit to fix. Should be good to go now. |
|
@srenatus anything else I can do on this? |
|
@jh125486 nothing left to do on this, except accept my gratitude 🎈 😉 Thanks for your contribution! |
4 participants
Why the changes in this PR are needed?
My company uses the OPA tooling internally, and updating the tablewriter package resolves TLM.
What are the changes in this PR?
casesto fix staticcheck SA1019Notes to assist PR review:
V1 tablewrite introduces a new syntax and also some differences in rendering, and I did my best to replicate the current outputs.
I worked with the tablewriter author to toggle some "features" for space merging, so that I could replicate the
prettyoutput line by line.I'm not sure why the max width of columns was being calculated in a few places... ideally we would let
tablewriteritself do the work.