search

rust-lang / jobserver-rs

Apache License 2.0
69 stars 39 forks source link

Fix `Client::configure*` on unix #100

Closed NobodyXu closed 1 month ago

NobodyXu commented 2 months ago

Fixed #99

the8472 commented 1 month ago

This also needs a regression test. Apparently nothing ensured that when inheriting a configuration from the parent we pass it through properly to a child.

NobodyXu commented 1 month ago

cc @the8472 Updated, added regression test as suggested.

NobodyXu commented 1 month ago

Also, I'd prefer presenting the regression test first in a commit, followed by the other commits actually fix the behavior and tests. That would make the first commit a minimal reproduction and we know we're fixing the right thing.

That might be a bit hard since I wrote after fixing it

NobodyXu commented 1 month ago

@weihanglo I squashed the commits, can't get the regression test commit to be the first, but it's much smaller now.

weihanglo commented 1 month ago

@petrochenkov

Would appreciate if you have time publishing this :)

(We should probably revisit https://github.com/rust-lang/jobserver-rs/pull/89 and https://github.com/rust-lang/infra-team/issues/117)