On Wed, Aug 15, 2012 at 10:12 AM, Stanislav Ochotnicky <sochotnicky@redhat.com> wrote:
Doing this with inheritance means you have to redefine run(), because you can't
have multiple is_applicable functions. And then would come the
inconsistency...in some cases it's enough to reimplement is_applicable, but in
other cases not. You can of course create another level of inheritance (as the
original Ruby checks do) to solve this problem

Decorators have a different problem and that is manual tests which still have
is_applicable. Since we got rid of it, it has to instead be changed to
automatic check which returns not_applicable when appropriate. Example:

Original:
class CheckLocale(GenericCheckBase):
    def __init__(self, base):
        ...
        self.automatic = False

    def is_applicable(self):
        return self.has_files('/usr/share/locale/*/LC_MESSAGES/*.mo')

New:
class CheckLocale(GenericCheckBase):
    def __init__(self, base):
        ..,
        self.automatic = True

    def run(self):
        if self.has_files('/usr/share/locale/*/LC_MESSAGES/*.mo'):
            self.set_passed('inconclusive')
        else:
            self.set_passed('not_applicable')


Merging is_applicable into run and lose the is_applicable method, looks fine to me, the original idea, what to test if a check was applicable and if it was, then run the check.
but integrating the check into run() would be good alternative too.
The decorator way is much harder to understand if you are not python decorator ninja master :)

Tim