Comment 4 for bug 488108

Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Thanks for reviewing, Jamu!

[1], [5], [6]

Fixed

[2], [3]

Yeah, I had noticed that as well, but I basically wanted to keep the default values of these "set" methods consistent with the values returned by the "get" ones. I agree it introduces unnecessary complexity though, so I changed the code as you suggest.

[4]

What about "added" and "removed" instead? The fact is that "lock" and "unlock" are actually further consequences the package locks themselves have on the packages, at this point we're really just reporting about newly "added" or "removed" package locks.

Another even better option for me would be "set" and "removed", which matches exactly the smart command line terminology, where you type things like:

smart flag --set lock "foo > 1.0"

and

smart flag --remove lock "foo > 1.0"

[7]

Absolutely a patch system would be better.

However we don't have any in place yet, and the code as it is now would actually succeed in upgrading an existent database, because the two CREATE TABLE statements are performed first, and they will succeed as the two tables don't exist yet. I guess a proper patch system would probably need a separate branch, I would personally postpone it till when it's actually necessary, I don't feel strong on this though.