API improvements

[minad]

Hi Hannes,

I played around with multi_git and I find the API quite hard to use (No comparison with rugged however). See https://github.com/minad/moneta/blob/2e9d5129ebc416089baac4026ed9f4ac44a6302d/lib/moneta/adapters/git.rb where I already use a modified version of multi_git (instance_eval - replaced by normal yield). I didn't upload it, I just tried to find a way how to make your lib more easy to use.

The things I find especially disturbing:

1. You overuse instance_eval, you should rather pass the builder to the block. Using instance_eval makes it really uncomfortable to access instance variables etc if you call it from a class.
2. After commit the new reference is returned. I would rather update the old one. The commit block should return the result of the block. This is also more consistent with how other libraries use blocks.
3. Committing should support an abort method, branch.commit {|c| c.abort } or something similar.
4. I find the builder scheme you use sometimes not very comfortable for example branch.commit {|c| c['file'].content } doesn't work. I would prefer if there wouldn't been so much distinction between builder and object. You might just want to modify an existing tree or file. I think this is similar what was done in the ruby-git and in my gitrb library.
5. Seemingly inconsistent use of Enumerable, Array/Hash-like APIs

Despite my criticism concerning the API I like the idea and I would really like to use this library. Looking forward to hear from you!

Daniel

[hannesg]

Hi Daniel

Thank you for mentioning!

1. Yep, this is on my todo-list. I think instance_exec looks cooler while yield is more usable in certain situations. I honestly want to support both.
2. I tried to make most things immutable unless it's clear that a user expects them to be mutable.
3. Anything that prematurely exits the block does that. I'm not sure if many people will use the method on the other hand it doesn't hurt anybody and shouldn't take much code.
4. Oh, that's definitely a Bug. I'll do my best to improve the interface. I guess the distinction is somewhat caused by the immutable references. A long as a reference is not mutable, writing something like repo,head['file']= 'content' is at least strange.
5. I'll do my best! Improvements welcome :)

Do you want commit rights?
Hannes

[minad]

Concerning 1. I would highly recommend to use just plain old yield and instance_eval ONLY in real DSL situations. You can give me commit rights if you want. Currently I don't have much time to work on it (a lot todo because of my phd) but it wouldn't hurt ;)

As a general hint I would just try to reduce the amount of code you are writing. What I have seen for now looks like a thoroughly designed system but some parts seem overly complex. Not that I can tell you now exactly what to improve, it is just a feeling and some things just didn't look simple when I tried to write this moneta adapter.

Daniel

[hannesg]

Okay, I'll give yield a try and see how it changes the overall appearance.

I really know what I'd like to improve: Ref and Tree::Base are currently pretty messy. Most code stems from strange inconsistencies between rugged and jgit. I hope I can clean it up.

BTW: I've rechecked branch.commit {|c| c['file'].content } from 0.0.1.rc1 and HEAD and it worked pretty well. Anything I miss?

Hannes