bedrock deployment get minor improvements

[samiyaakhtar]

• Default top deployments to 50, to match default in spektate dashboard
• A flag to override having no separators in the table view, but show separators by default
• Introduce state column to normal mode but remove start time column, and also added duration column
• Truncated some fields on normal mode to adjust the addition of new columns, but no truncation in wide mode
• Introduced author to be shown in wide output, since it adds api calls
• Refactored some messy table printing code in get.ts and moved it to lib/deploymenttable.ts

See comments for updated screenshots

Related to microsoft/bedrock#743
Smoke test

[andrebriggs]

@samiyaakhtar Looks good over all but somehow we decreased code coverage. Can we at least equal what we had before?

[dennisseah]

you have added two new options --top and --remove-separators and we do not see unit tests for them.

[samiyaakhtar]

you have added two new options --top and --remove-separators and we do not see unit tests for them.

@dennisseah There are unit tests for these, but they're not at command level (just like for all other flags)

[dennisseah]

you have added two new options --top and --remove-separators and we do not see unit tests for them.

@dennisseah There are unit tests for these, but they're not at command level (just like for all other flags)

how can we test if 10 entries are returned if we do --top 50? what about negative test? --top x or --top -1. and the show and hide separators, are separators shown or hidden respectively?

[gemorris]

I am confused about the following item:

• Introduce state column to normal mode but remove start time column, and also added duration column

Why was the start time column removed? Did we have user feedback about this? My concern is that the rows are now disconnected from time (It looks like we have end time in the wide version though?)

Would like to understand the thinking here.

[samiyaakhtar]

@gemorris From our last discussion about this a while ago, we decided that the real estate being used by start time could be utilized better, in both wide and narrow outputs, so we had decided to bring in author, PR and merged by to utilize that real estate. I could bring back start time but the time strings are long. Let me know if it should be brought back :)

[samiyaakhtar]

A few minor improvements:

• All cells are left aligned except for numbers
• Dashes were being used as fillers, removing them as they create noise, now that we have an explanation for empty cells ("Manual HLD Edit")
• Introduced start time in wide column instead of end time, since it may add more value than end time

Let me know your feedback @gemorris @andrebriggs @timfpark

[samiyaakhtar]

@andrebriggs @gemorris
As discussed, UX for Manual HLD Edit and support for csv output formats will come in a later PR (see here). Right aligned Duration column.

Please give feedback so we can close this out :)

[andrebriggs]

@samiyaakhtar so you're saying you want to show the HLD only edits by default for now?

[samiyaakhtar]

@andrebriggs For now, yes, until we work on the item Flag for manual hld edits, disabled by default, so that the default display is just deployments that are bedrock initiated in microsoft/bedrock#743

The other reason is that it would be nice to explore colors and custom fonts in this library as part of a separate PR effort, so that "Manual HLD Edit" is shown in a more user friendly manner