Skip to content

Make summary table vertical #1334

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

Merged
merged 1 commit into from
May 25, 2022
Merged

Make summary table vertical #1334

merged 1 commit into from
May 25, 2022

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented May 24, 2022

Based on Zulip discussion. The current look of the table can be seen here.

The table is now vertical.

I refactored the code to build a sort of a 2D matrix of values (Vec<Vec<String>>), so that it is easier to change the horizontal/vertical layout or column/row order in the future.

@Kobzol Kobzol requested review from rylev and Mark-Simulacrum May 24, 2022 09:54
Copy link
Member

@rylev rylev left a comment

Choose a reason for hiding this comment

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

I like the layout change, but most of the changes to the content of the cells don't feel like improvements to me...

@Kobzol Kobzol force-pushed the summary-table-vertical branch from 4d511a5 to 474100b Compare May 24, 2022 13:22
Copy link
Member

@rylev rylev left a comment

Choose a reason for hiding this comment

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

Looks good. Just one thing to address about row width.

Copy link
Member

@rylev rylev left a comment

Choose a reason for hiding this comment

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

Let's merge and get some experience with this

@rylev rylev merged commit 863d2be into master May 25, 2022
@rylev rylev deleted the summary-table-vertical branch May 25, 2022 07:57
nnethercote added a commit to nnethercote/rustc-perf that referenced this pull request Oct 28, 2022
The table spacing was made nice in rust-lang#1312 but then got messed up again
in rust-lang#1334.

This commit does the following.
- Fixes the table spacing.
- Changes how test failures are printed, so the expected and actual
  outputs are printed nicely across multiple lines, instead of being
  stringified with into a single line. I should have done this some time
  ago, it makes adjusting the expected output *so* much easier.
- Renames some variables, for clarity and consistency.
- Imports `std::cmp` because it's used in multiple places.
- Other minor tweaks.
nnethercote added a commit to nnethercote/rustc-perf that referenced this pull request Oct 28, 2022
The table spacing was made nice in rust-lang#1312 but then got messed up again
in rust-lang#1334.

This commit does the following.
- Fixes the table spacing.
- Changes how test failures are printed, so the expected and actual
  outputs are printed nicely across multiple lines, instead of being
  stringified with into a single line. I should have done this some time
  ago, it makes adjusting the expected output *so* much easier.
- Renames some variables, for clarity and consistency.
- Imports `std::cmp` because it's used in multiple places.
- Other minor tweaks.
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