Hi Corey,
I had started working on this a looooooong time ago (when Bodhi 2 was still supposed to be written with TG2 ^^') but due to lack of time I had to stop. [1]
So I'm very happy you picked it up. :)
One thing you've done and that I wanted to do was to use an Enum for the Karma types. I had made it with a clumsy database table, and it felt dirty.
Now if I may, I'd like to offer a quick review of your patch.
config.add_view('bodhi.views.view_model', - accept='text/html', context='bodhi.resources.BodhiResource', + accept='text/html', renderer='bodhi:templates/model.mak')
Why change the order here?
-from sqlalchemy.orm import scoped_session, sessionmaker, relation, relationship +from sqlalchemy.orm import scoped_session, sessionmaker, relation [... snip ...] - bugs = relationship('Bug', secondary=update_bug_table, - backref='updates', lazy=False) + bugs = relation('Bug', secondary=update_bug_table, + backref='updates', lazy=False)
This is backwards: """relationship() is historically known as relation() prior to version 0.6.""" http://www.sqlalchemy.org/docs/orm/relationships.html?highlight=relation#sql...
So if relationship is the new name, it might mean that relation could get deprecated in the future. If you want to unify it all, I'd say it would be better to use relationship everywhere?
Other than that, your implementation seems a bit too simplistic compared to what QA had asked. Correct me if I'm wrong, but it seems that each comment can only add one type of karma, whereas QA seemed to want: - some karma related to the update (which I had called global karma in my branch) : still works for me, fails for me, passes test case, introduces new errors,... - some karma related to the bugs the update is supposed to fix: fixes bug 1234 as advertised, doesn't fix bug 1234 - one comment could combine one global karma and one karma per bug linked to the update (see screenshots from my attempt) [1]
All in all, I think you should have a look at what I had done, as I had spent quite some time to make sure that: - it was possible to make one comment with different types of karma - email bodies would show all the karmas as expected
I'm not saying you have to take it as is, it's been done a long time ago (and I don't think my opinion is authoritative for Bodhi anyway). However it could certainly give you a few ideas. Perhaps you could even just port it to use an enum as you have done here and everything would work great. (I doubt it though, that would mean I actually wrote some good code :P)
In any case Luke, for the automatic stable and unstable karma handling, what is needed is a policy from FESCo (or even just some draft guidelines from QA) to write the rules, as they will obviously be more complicated than just a numeric threshold. This is most of what I had written in the TODO paragraph on the page. [1]
I guess it's time to revive those discussions with QA and FESCo?
Again, thanks for taking over this, I kept wanting to go back to it but haven't gotten the time to in more than a year now.
Cheers,
[1] http://bochecha.fedorapeople.org/bodhi_karma_revamped/#todo