persistent
persistent copied to clipboard
polux
vacuum-persistent
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
@tomaskulich thank you for this even more impressive PR :) As discussed on #7, please let me know when everyone has signed the CLA. As you can see, there's also now a googlebot tracking this.
@polux all relevant people have signed the CLA. Also, feel free to change / refactor / discuss anything.
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.
Hm, googlebot doesn't seem happy with the CLAs :) I'm not sure, what to do about it.
Thank you! I'll check manually that everyone as signed the CLA then. It'll take me some time to merge the PR as there are apparently merge conflicts, but I'll do it ASAP. Thanks again!
@tomaskulich we're trying to understand why googlebot rejects the PR and would need you to ping this thread to trigger it again while we monitor it. Could you please ping this thread when you get a chance? Sorry for the inconvenience.
@tomaskulich I just go an explanation from the googlebot team: since you opened the PR it's clear that you're ok with this code being contributed to Google. But even if your co-authors have signed the CLA, there's no way of knowing they're ok with this particular code being contributed to Google. They might have signed the CLA for an unrelated project. There is no way for googlebot to know they're ok even if we do.
@syslo, @matystl, @jurom, @brandys11, @black3r can you please +1 this thread or say "I agree" or something like that?
Even after I get all the +1s googlebot won't mark the PR as successful because it can't parse "any form of agreement", but I will know it's ok to merge.
Thank you again for this huge pull request!
+1 and "I agree" not realy sure what +1 means so hope comment like this is enough
"I agree", +1, and I'm ok with this particular code being contributed to Google. I hope this statement is sufficient.
Thanks guys, "+1" is enough, "I agree" is enough too. So what you wrote is plenty enough :)
Yay! Thank you all again for this amazing pull request! It apparently has conflicts so it'll take me some time to merge but I'll make sure to do it. My plan is to basically release a version 1 which is your version 2.
@polux Hi, how is it going? Did you have a chance to go through the PR?
Hi, I've started looking at it but I've had a lot of work recently and it's a big PR :) My intent though is to discuss a few design points with you and then make it the basis for version 1 of the library.
Here's a first question: did you chose to define
V get(K key, [V notFound])
instead of
V get(K key, [V notFound()])
on purpose? I would have gone for the latter but I can imagine that someone would find it overkill. Did you give it some thought and then chose the latter or did you just pick the former without considering the alternatives? In that case, would you think it makes sens to change it?
Hi, we were discussing both of the alternatives. I agree that the second alternative gives you more power, but its questionable, whether it's worth the cost. The two use-cases which come to my mind are:
Use case 1: Obtaining the notFound value takes a long time, i.e.
cache.get('key', timeConsumingOperationToGetValue)
seems more reasonable than
cache.get('key', timeConsumingOperationToGetValue())
Use case 2: Obtaining notFound value may fail, if key is already present in the map. In such a (rare) case:
map.get('key', mayFailWhenMapContainsKey)
will be safe although
map.get('key', mayFailWhenMapContainsKey())
won't.
We consider these use-cases as quite unimportant (you can always use a workaround with setting notFound to null / none / whatever and then calculate the alternative value explicitly) and not worth the additional typing.
@polux Please let me know, if there is anything else, I can explain. There are a few weird issues (hopefully, they are properly commented) in the code, mostly because we tried to tune the performance and this comes at some cost.
@polux I realized, that some parts of the code must be really hard to understand without 'bigger picture' of how the implementation work. I wrote technical documentation (in separate file, just commenting the code felt strange) explaining a few things, maybe you'll find it useful.
Hey guys, just wanted to express interest in a consolidated persistent 1.0 :) Thanks for working on this.
I haven'y forgotten about this but this PR is huge and:
- it has merge conflicts
- it has some breaking changes (return type of lookup for instance)
I need a large chunk of time to work on merging it, maybe my next vacation...