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

Add missing hashbrown drain APIs for Map and Set #19

Merged
merged 5 commits into from
Jun 19, 2021

Conversation

pedromfedricci
Copy link
Contributor

Related to #4 - "The Drain iterators".

@pedromfedricci
Copy link
Contributor Author

Hmm guess tarpaulin is compiling tests with rustc < 1.52.0?
Related to this change: Eventually adopt rustdoc::all change

@jonhoo
Copy link
Owner

jonhoo commented Jun 6, 2021

Hmm, I didn't think tarpaulin was using an old compiler..
@xd009642 do you know what might be causing this error?

   Compiling griddle v0.5.1 (/__w/1/s)
error: Broken pipe (os error 32)
warning: build failed, waiting for other jobs to finish...
thread 'main' panicked at 'already borrowed: BorrowMutError', src/tools/cargo/src/cargo/util/config/mod.rs:307:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
May 31 18:59:56.938 ERROR cargo_tarpaulin: Failed to compile tests! Error: griddle: an unknown tool name found in scoped lint: `rustdoc::all`

src/set.rs Show resolved Hide resolved
@xd009642
Copy link

xd009642 commented Jun 6, 2021

I'm not sure I'd have to look at the tool attributes and CI jobs, but I can say tarpaulin just uses the version of the compiler installed in the system as it calls out to the process. So either the rust version used in the tests is old or there's something else afoot 🤔 . I raised an issue in tarpaulin and I'll try to have a deeper look this week

@jonhoo
Copy link
Owner

jonhoo commented Jun 6, 2021

So, this is specifically running with the Docker image (specifically latest), so there won't be anything in the environment beyond what that sets up.

@xd009642
Copy link

xd009642 commented Jun 6, 2021

Okay that will be the latest stable compiler (at time the image was built) then

@jonhoo
Copy link
Owner

jonhoo commented Jun 6, 2021

That's weird though, as the latest stable compiler should support rustdoc::all, which seems to be what it's failing on 🤔

@xd009642
Copy link

xd009642 commented Jun 7, 2021

@jonhoo latest hadn't been updated since 1.51.0, a new tarpaulin:latest is now built and pushed with 1.52.0 👍

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2021

Codecov Report

Merging #19 (26c8daf) into master (72b4b85) will increase coverage by 0.41%.
The diff coverage is 77.55%.

Impacted Files Coverage Δ
src/set.rs 56.52% <70.00%> (+2.02%) ⬆️
src/map.rs 64.11% <89.47%> (+0.77%) ⬆️
src/raw/mod.rs 70.91% <0.00%> (+1.19%) ⬆️

@jonhoo
Copy link
Owner

jonhoo commented Jun 10, 2021

That seems to have done it, thanks!

@pedromfedricci Just the change from the comment left now and then I think we're 👍

@pedromfedricci
Copy link
Contributor Author

Hey! @jonhoo
I've made a few adjustments, do you see anything else I'm missing?
ty

Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

This is excellent, thank you!

@jonhoo jonhoo merged commit a0bd382 into jonhoo:master Jun 19, 2021
@pedromfedricci pedromfedricci deleted the issue-4 branch December 22, 2022 03:14
jonhoo pushed a commit that referenced this pull request Oct 19, 2024
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.

4 participants