IreneKnapp / direct-sqlite

MIT License
35 stars 54 forks source link

Assorted improvements #58

Closed redneb closed 9 years ago

redneb commented 9 years ago

This pull request contains a bunch of assorted additions to the library. I tried to break this into multiple PRs but this resulted in a merge conflict hell. I think it is easier to merge as a single PR and you can still review the individual commits. If you prefer multiple PRs, I can do it.

Here's an overview of the commits:

nurpax commented 9 years ago

Looks very good in general, thanks for your contribution! No need to split into separate PRs.

You have really good information in your PR description some of which is not available in commit messages or in code comments. E.g.

This is useful when you want to manually control when checkpointing occurs in wal journal mode. Unfortunately, it is not possible to provide high-level bindings to this function in haskell without leaking a FunPtr. Thus I only provide the low-level bindings and let the user manage the allocation/deallocation of the FunPtr.

I think this could go into the function doc comment too? Similarly, it wouldn't hurt for this to be in commit messages. Commit message is what many people will read when doing code archaelogy later. It's not easy to go back to PR descriptions from individual commits.

I will read this in more detail later today/this week, but thus far I don't have any complaints. I'm glad you also added some tests for these new features!

redneb commented 9 years ago

I can certainly add that information to the commit messages but that would require rebasing and sending a new PR. So I will hold this off just in case you have more comments.

IreneKnapp commented 9 years ago

Oh, hey! The online backup API! Thank you! That's been a highly-wanted for quite some time. (In fact, there's an open issue asking for it.)

nurpax commented 9 years ago

Great work, I reviewed all the changes now and I don't have any complaints. Thank you very much for this contribution.

If you want to follow up with docs changes, for example for the wal_hook comment, feel free. But I think this is releasable as is, and I will soon upload this on hackage.