Hello, Am Freitag, 5. Juni 2015 schrieb Steve Beattie: > On Wed, Jun 03, 2015 at 11:57:00PM +0200, Christian Boltz wrote:
> > [ 19-baserule-add-severity.diff ] > > Acked-by: Steve Beattie <[email protected]> as is. Some comments: > > === modified file utils/apparmor/rule/__init__.py > > + def severity(self, sev_db): > > + '''return severity of this rule (a number between 0 and 10, > > where 0 means harmless and 10 means critical), + or > > sev_db.NOT_IMPLEMENTED if no severity check is implemented for this > > rule type. > Should the comment here say anything about possibly returning > sev_db.unknown? Should I add another line to the comment mentioning "unknown"? (to make things more interesting, the exact value of "unknown" is specified when creating a sev_db instance, the default for unknown is "10", and everywhere we use sev_db we have _("unknown") as default) > > === modified file utils/test/test-baserule.py > > + def test_default_severity(self): > > + sev_db = severity.Severity('severity.db', 'unknown') > > + obj = BaseRule() > > + rank = obj.severity(sev_db) > > From an object-oriented perspective, it feels a little odd to me to > ask a rule what the severity of that rule is given a severity db. I see the severity as a property of the rule, which is one of the reasons why I placed it in the rule. (sev_db is just the tool helping to find out the severity, and the main reason why I pass in the sev_db is that I don't want to have it as global variable somewhere.) Another reason was that the check needs to access a class-internal variable, and it felt more natural to have access to that variables inside the class (and then ask sev_db about a string). Summed up "Dear rule, how severe are you?" > It > seems like the more natural conceptual structure would be to ask the > severity db what it thinks the severity of a given rule object is. Not > an objection to this patch, but just one of those things that feels > quirky about the division of responsibilities. Summed up: "Dear sev_db, how severe is this rule?" OK, that sounds like a reason to move the code to severity.py. (_One_ reason. I have two *eg*) Can we agree to disagree, or do we need to vote in the next meeting? ;-) Regards, Christian Boltz -- > My personal opinion is that bugzilla should be sent in a rocket > to the oblivion and pick up something sane and usable, [...] Do you really mean the core Bugzilla software or perhaps some infrastructure software like this "Internet in CHAINs" thingy or whatever its actual name is? [> Cristian RodrÃguez and Johannes Meixner in opensuse-factory] -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
