Skip to content
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.

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

feat(cli): format dataset copy output with console-table-printer #3303

Merged

Conversation

tzhelyazkova
Copy link
Member

@tzhelyazkova tzhelyazkova commented May 24, 2022

Description

Change the formatting of the dataset jobs list output.
Introduces a new npm dependency console-table-printer

With this library we are able to get rid of the index column and the quotes around the values in the table, which are present in the solution with console.table()

Screenshot 2022-06-08 at 16 07 49

@vercel
Copy link

vercel bot commented May 24, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
perf-studio ✅ Ready (Inspect) Visit Preview Jun 8, 2022 at 2:12PM (UTC)
studio-workshop ✅ Ready (Inspect) Visit Preview Jun 8, 2022 at 2:12PM (UTC)
test-studio ✅ Ready (Inspect) Visit Preview Jun 8, 2022 at 2:12PM (UTC)

Comment on lines 12 to 15
table(...args) {
console.table(...args)
},

Copy link
Member

Choose a reason for hiding this comment

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

Given that we are using console-table-printer, I don't think this change is needed?

Returns table of jobs with:
- jobId
- current state - pending, completed, failed, terminating, terminated
- History - With/Without history
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- History - With/Without history
- history - whether or not dataset copy included document history

}
}

function convertTimestampToDaysHoursMinsSecs(timePassedInSeconds) {
Copy link
Member

Choose a reason for hiding this comment

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

Can I suggest using shorthand formatting for these, to compress the table some more? Eg instead of 3 days 22 hours 32 minutes 6 seconds it would be 3d 22h 32m 6s

}

if (response && response.length > 0) {
const p = new Table({
Copy link
Member

Choose a reason for hiding this comment

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

I think table would be a more descriptive variable name

@rexxars rexxars changed the title feat(dataset copy list): Format output with console-table-printer feat(cli): format dataset copy output with console-table-printer Jun 1, 2022
@tzhelyazkova tzhelyazkova force-pushed the feature/sc-18247/list-job-history-sanity-cli branch from b7c6250 to 2eed1ac Compare June 1, 2022 13:52
@tzhelyazkova tzhelyazkova force-pushed the feature/sc-18247/list-job-history-sanity-cli-formatting branch from 143e0fe to 7275b86 Compare June 1, 2022 13:57
@tzhelyazkova
Copy link
Member Author

tzhelyazkova commented Jun 1, 2022

@rexxars this PR is actually supposed to be only the one commit about formatting, since it proved to be a bigger topic of discussion in our team. But it's based on top of #3292 so whenever there are changes in #3292 this PR gets messed up.
I implemented your suggested changes in #3292.

@rubioz rubioz self-requested a review June 1, 2022 14:20
@rexxars
Copy link
Member

rexxars commented Jun 1, 2022

@rexxars this PR is actually supposed to be only the one commit about formatting, since it proved to be a bigger topic of discussion in our team. But it's based on top of #3292 so whenever there are changes in #3292 this PR gets messed up. I implemented your suggested changes in #3292.

Ah, sorry - did not have that context.

@tzhelyazkova tzhelyazkova force-pushed the feature/sc-18247/list-job-history-sanity-cli branch from 2eed1ac to 3ff9735 Compare June 2, 2022 10:06
@RitaDias RitaDias force-pushed the feature/sc-18247/list-job-history-sanity-cli branch 2 times, most recently from a488dff to 61cf425 Compare June 8, 2022 13:16
Base automatically changed from feature/sc-18247/list-job-history-sanity-cli to next June 8, 2022 13:40
@tzhelyazkova tzhelyazkova deleted the feature/sc-18247/list-job-history-sanity-cli-formatting branch June 8, 2022 14:26
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.

None yet

5 participants