rename covers to is_start_of
#57
Conversation
|
The failing tests are unrelated to this change. They are due to rust-lang/rust@671b2a5 which will be fixed by rust-lang/rust-clippy#2713. |
foundationdb/src/subspace.rs
Outdated
| /// indicating that the Subspace logically contains the key. | ||
| pub fn covers(&self, key: &[u8]) -> bool { | ||
| pub fn starts_with(&self, key: &[u8]) -> bool { |
There was a problem hiding this comment.
My concern, and the reason I didn't use starts_with, is that usually the semantics of starts_with is to take a prefix and see if the self has that at it's start, in this implementation we're checking if the key starts with self.
This is why I was hesitant to use this before. We discussed an other option, but @yjh0502 was less enthused with it: is_prefix_of. Another option would be is_start_of.
Does my concern make sense?
There was a problem hiding this comment.
I didn't realized that point. I'm okay with all those names, but if I have to choose one of those, I prefer covers :)
There was a problem hiding this comment.
in this implementation we're checking if the key starts with self
Ah, I see. That, indeed, makes sense. In that case I think I like is_prefix_of or is_start_of more than covers because to me covers is a bit vague. Those 2 explain the relationship more clearly.
There was a problem hiding this comment.
so splitting the difference, should we just land on is_start_of?
There was a problem hiding this comment.
I like that. Let me update the PR.
EDIT: Updated as per conversation with @bluejekyll and @yjh0502.