More bugfixes & some suggested changes

[matheus23]

In this PR the individual commits should read really nicely!
So I'd recommend reviewing by commit.

There's mostly bugfixes, some solved FIXMEs, which I'd expect be uncontroversial changes (ignoring possible bugs I've introduced).
But there's also some changes that are more subjective:

• Take command: &str instead of command: String in Store::get_chain: I think this makes sense, because get chain is mostly a lookup, it doesn't need ownership to construct the return value.
• Turn the Iterator::try_fold call in Invocation::check into a for loop: I think that just reads slightly easier & is more idiomatic in rust. reverted this, it's in the history should we need it :)

[expede]

Turn the Iterator::try_fold call in Invocation::check into a for loop: I think that just reads slightly easier & is more idiomatic in rust.

Aren't they both idiomatic? The docs seem to imply that they are, at least — possibly even encouraging folds as I read it 🤔 Googling around, it looks like there's some debate about which to use. Stylistically, I like that try_fold expresses its intention without having to read the entire code block.

I'm sure that there's cases where each does better, but I've also seen a few benchmarks where iterator methods like fold outperform loops (maybe because they're more structured?)

[matheus23]

Okay, sure. I personally found it harder to read, but I have pretty weak opinions on that one.

[expede]

but I have pretty weak opinions on that one

Oh yeah, same here. I was just surprised by the claim!

[expede]

To account for starvation I guess?

[matheus23]

Yeah, due to this conversation. I think RwLocks being able to cause deadlocks is... not great. Defaulting to Mutex seems like the better option 👍

[expede]

Thanks @matheus23

[matheus23]

I only rebased the same changes on the newest commits from the v1.0-rc1 branch, so the changes you reviewed are the same!
Also ran cargo test --all-features and that is happy, so merging this ✌️