Convert Storage::entries's max_size argument to Option<u64>

[bkchr]

Currently the value is just a u64 with u64::MAX defined as NO_LIMIT.
This no limit is not mentioned in the documentation. Something like the following enum would suite better:

enum Size {
    Fixed(u64),
    NoLimit,
}; 

I could provide a PR, if wanted.

Edit (brson): per discussion, let's change this to Option<u64> with a doc-comment explaining that None means "no limit". The Storage trait is in src/storage.rs.

[siddontang]

Maybe adding a comment is better.

/cc @Hoverbear

[Hoverbear]

@bkchr Would adding a comment about this limit be satisfactory? Since NoLimit == Fixed(u64::MAX)

[bkchr]

Yeah I know that NoLimit == Fixed(u64::MAX) after looking into the code. I'm unsure. I think a comment would be a good start if you don't want to add an extra enum. However, adding an enum would be the cleaner solution in my point of view.

[Hoverbear]

Perhaps Option<u64> is appropriate. A None value correctly communicates that The max size is none.

[bkchr]

Yeah, an option would also fit here ;)

[Hoverbear]

@bkchr Would you still be interested in making a PR, or is this open for others to tackle?

[bkchr]

Open for others :)

[estk]

Id like to work on this!

[estk]

dang, too slow :/

[pwoolcoc]

sorry @estk, I should have commented when I started working on it :-/

if you are so inclined, I'd love some code review of my PR!

[estk]

Sure thing @pwoolcoc, you're too quick on the draw :)

[Hoverbear]

@estk Aw, thanks so much! Do you think you might be interested in this one instead? #70

[estk]

@Hoverbear haha, you really gotta be quick here! Thanks for pointing that one out, but it looks like I'm too late again. I'll be away for the weekend but I'll swing back next week and try to pick up an issue. Thanks for the hospitality @Hoverbear

[Hoverbear]

🤦‍♀️ Ha, I'm sorry. :)