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

benches: move to criterion-rs and GitHub actions #366

Merged
merged 1 commit into from Nov 10, 2020

Conversation

lucab
Copy link
Member

@lucab lucab commented Nov 6, 2020

This moves all benches to criterion-rs, in order to allow running
benchmarks on a stable toolchain.
This also moves benchmarking and linting jobs to a pinned toolchain,
under GitHub actions.

@lucab
Copy link
Member Author

lucab commented Nov 6, 2020

This was green on my fork: https://github.com/lucab/rust-prometheus/actions/runs/349778558

Copy link
Contributor

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

I am in favor of this. Thanks for doing the change.

.github/workflows/rust.yml Outdated Show resolved Hide resolved
.travis.yml Outdated
@@ -46,6 +40,5 @@ script:
cargo test -p prometheus-static-metric --no-default-features --features="$FEATURES"

# Run benchmarks.
cargo bench --no-default-features --features="$FEATURES"
cargo bench -p prometheus-static-metric --no-default-features --features="$FEATURES"
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit reluctant to only partially move to criterion. Is there something blocking us from using criterion in prometheus-static-metric as well?

Copy link
Member

@breezewish breezewish Nov 9, 2020

Choose a reason for hiding this comment

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

Maybe because prometheus-static-metric itself is something that requires nightly?

Copy link
Member Author

@lucab lucab Nov 9, 2020

Choose a reason for hiding this comment

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

No sorry, I just ran out of time on Friday and preferred to split the move into smaller bits. I saw today that there wasn't too much left to be converted, so I amended this PR now to complete the move.

This moves all benches to criterion-rs, in order to allow running
benchmarks on a stable toolchain.
This also moves benchmarking and linting jobs to a pinned toolchain,
under GitHub actions.

Signed-off-by: Luca BRUNO <luca.bruno@coreos.com>
@lucab
Copy link
Member Author

lucab commented Nov 9, 2020

This should be ready for another round of reviews.

Checks will not show up here until the workflow is merged first, but they were green on my fork: https://github.com/lucab/rust-prometheus/actions/runs/353547239

@lucab lucab merged commit 200c362 into tikv:master Nov 10, 2020
@lucab lucab deleted the ups/criterion-benches branch November 10, 2020 11:03
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

3 participants