Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.Sign up
New History APIs #632
mhammond left a comment •
This looks fine to me although I'm not convinced we should expose
Do you think we can split that from this patch into a new PR and hand-ball it to Alex and/or Ryan?
Hmm, sure. Extracting it isn't a problem, but I expect we'll get (somewhat deserved) pushback. Again, it's only a problem for sync users, which aren't all (or even most) users.
I also disagree that wiping the server is the right choice. That will leave most of the visits on remote devices.
If we're going that far, what we might really want to wipe the server and also send a clients engine command to tell the other devices to wipe their clients, maybe.
And I don't think we can realistically say that we won't implement deleteEverything until we e.g. implement the clients engine so we can send that command.
Edit: That said, I don't mind removing it. Its absence shouldn't block anyone as they can just call the range delete manually (which is all it does).
Yeah, fair enough. I'd really just like explicit buy-in from product on this if possible.
That's probably true too, but I'd rather ensure we are explicit about this decision than it being an "accidental implementation detail"