Customizable rules #8

Closed
indrekj opened this Issue Apr 1, 2013 · 6 comments

3 participants

@indrekj
Collaborator

Hi,

I find yardstick very useful. But I'm not really a fan of the "@api" tag validation. What is your take on the customizable rules like the reek gem has? I'm okay with implementing this if it is a good idea. I'm thinking something like this:

---
ApiTag:
  enabled: true
@dkubb
Owner

@indrekj I think reek's approach is better than most metrics tools. You need ways to turn something on/off globally, as well as specifying exceptions to the rule when you need to:

---
ApiTag:
  enabled: true
  exclude:
     - Foo#bar
     - Baz.qux

My only problem with reek is that the labels used in the configuration sometimes, but don't always, relate to the warning that is output. You either have to memorize the mapping, or grep through the reek source to find out which setting in the config file a specific warning corresponds to.

Ideally, I would've structured yardstick so that each of the rules is handled by one class, and the class could correspond to the labels in the config file. Right now it's structured like this: https://github.com/dkubb/yardstick/blob/master/lib/yardstick/method.rb#L7-L10 ... As an interim approach, perhaps we could finalize the names of the rules for the config files and modify #rule so the first parameter is the label and the second parameter is the human readable description.

If you think this is the right approach, do you have some suggested names for each rule that we could discuss before you implement it?

@dkubb
Owner

While we are discussion exclusion rule formatting, I would probably want to support 3 types:

---
exclude:
  - Foo      # class or module
  - Foo#bar  # instance method
  - Foo.bar  # class method
@indrekj
Collaborator

I was also thinking about one class per rule, but my initial idea was to group similar rules. e.g Rules::Summary could check the presence and length of a summary.

I haven't thought about names yet. Maybe something like ApiTagPresence, ApiTagValueOfPrivateMethod, ApiTagValueOfProtectedMethod. There's probably something better.

@mbj
Collaborator
mbj commented Apr 2, 2013

@dkubb Do you see how to gemify this kind of fine grained configuration somehow? I'd love to save time with not replicating it in mutant.

@dkubb
Owner

@indrekj thanks for the PR! I just merged it and am closing this issue.

@dkubb dkubb closed this May 3, 2013
@mbj
Collaborator
mbj commented May 3, 2013

@indrekj Also thx from my side. Your changes make it more easy to "sell" devtools workflow to development teams ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment