jonhoo / griddle

A HashMap variant that spreads resize load across inserts
Apache License 2.0
190 stars 7 forks source link

Add missing hashbrown drain APIs for Map and Set #19

Closed pedromfedricci closed 3 years ago

pedromfedricci commented 3 years ago

Related to #4 - "The Drain iterators".

pedromfedricci commented 3 years ago

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

jonhoo commented 3 years ago

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`
xd009642 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

jonhoo commented 3 years ago

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

xd009642 commented 3 years ago

@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 commented 3 years ago

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%) :arrow_up:
src/map.rs 64.11% <89.47%> (+0.77%) :arrow_up:
src/raw/mod.rs 70.91% <0.00%> (+1.19%) :arrow_up:
jonhoo commented 3 years ago

That seems to have done it, thanks!

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

pedromfedricci commented 3 years ago

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