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

sort sub-benchmarks by name (fixes #179) #180

Merged
merged 1 commit into from
Aug 4, 2018

Conversation

mykmelez
Copy link
Contributor

This sorts the sub-benchmarks by name when generating an HTML report, resolving #179 (assuming you agree that we should fix that! I know that this PR is putting the cart before the horse, as we haven't had any discussion in that issue yet, but I figured it would be a small change, and I wanted to familiarize myself with the codebase).

Note that CONTRIBUTING suggests to run cargo +nightly fmt, but that currently fails for me with:

thread 'main' panicked at 'not yet implemented', tools/rustfmt/src/expr.rs:346:47

Which is rust-lang/rustfmt#2743. However, I don't foresee any formatting issues with this small (two-line) change. And in any case rustfmt makes a bunch of other changes to the code before panicking, so it looks like the codebase hasn't been getting formatted lately (or at least not with recent versions of rustfmt!).

@coveralls
Copy link

Pull Request Test Coverage Report for Build 425

  • 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.004%) to 12.105%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/html/mod.rs 0 2 0.0%
Totals Coverage Status
Change from base Build 424: -0.004%
Covered Lines: 410
Relevant Lines: 3387

💛 - Coveralls

@bheisler bheisler merged commit 1f677f1 into bheisler:master Aug 4, 2018
@bheisler
Copy link
Owner

bheisler commented Aug 4, 2018

Thanks for the pull request!

@mykmelez mykmelez deleted the html-sort-order branch August 6, 2018 16:29
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