Code review comment for lp://qastaging/~vila/bzr/lockable-config-files

> 305 + @classmethod
> 306 + def from_bytes(cls, unicode_bytes):
>
> It seems a little weird to call it 'unicode_bytes' and to have the function be
> 'from_bytes'. What about 'from_unicode(cls, unicode_content)' ?

I just went with what spiv proposed.

I changed it to from_string(str_or_unicode).

>
> I also thought that 'lock_write' would take care of reload(), so that callers
> couldn't actually get it wrong. (reload whenever the lock transitions from 0
> => 1 lockers.)

Hmm, that may be worth considering, but it's outside the scope of this bug.
I've not change the over data model here and forcing the call to reload
may block some use cases. What if I want to inspect the values already known
by the config before reloading from disk (which must happen inside the lock
scope) ?

The constraint to call reload() (which is new, like LockableConfig) shouldn't
be a such burden for new adopters and could even help them understand what
is really happening here (also note that I only needed to add *3* calls and
that reload() usage is more than hinted in LockableConfig docstring).

> This seems ok enough to land, but it also seems like it could be improved.

What can't ? But since this is my third submission for this bug, I think
I've spend enough time on it ;) I intend to keep working in the config area
anyway.