dkubb / yardstick

A tool for verifying YARD documentation coverage
http://wiki.github.com/dkubb/yardstick
MIT License
171 stars 22 forks source link

Customizable rules #9

Closed indrekj closed 11 years ago

indrekj commented 11 years ago

Discussed in #8

This at the moment already works, but still needs a bit more work.

This is the default config file:


---
threshold: 100
rules:
  ApiTag::Presence:
    enabled: true
    exclude: []
  ApiTag::Inclusion:
    enabled: true
    exclude: []
  ApiTag::ProtectedMethod:
    enabled: true
    exclude: []
  ApiTag::PrivateMethod:
    enabled: true
    exclude: []
  ExampleTag:
    enabled: true
    exclude: []
  ReturnTag:
    enabled: true
    exclude: []
  Summary::Presence:
    enabled: true
    exclude: []
  Summary::Length:
    enabled: true
    exclude: []
  Summary::Delimiter:
    enabled: true
    exclude: []
  Summary::SingleLine:
    enabled: true
    exclude: []

It can be used like this:

  config = YAML.load_file(File.expand_path('../../../config/yardstick.yml', __FILE__))
  Yardstick::Rake::Verify.new(config)

I am interested to know if you don't like something or want something to be changed.

Also, I followed the DM2 coding style and have only tested it with ruby 1.9.3.

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling b4cd9b38f3793a1efca8195f5a949e63e404c04e on indrekj:customizable-rules into c0113cb7611fcecb8cdfc3ce05380da92856e526 on dkubb:master.

mbj commented 11 years ago

@indrekj What is the motivation behind that change? I love to be forced to write all documentation ;)

snusnu commented 11 years ago

@mbj imo "all" is subjective, i like the idea behind that PR!

mbj commented 11 years ago

@snunsu, @indrekj Do not misunderstand me. I support that PR intentions also, just like to learn how @indrekj defines "all".

indrekj commented 11 years ago

@mbj, I think every rule should be enabled when dealing with open source projects and gems. I'd also like to use it in my Rails projects, but I don't want to write @api tags everywhere, because ruby's usual public/protected/private keywords are usually enough.

snusnu commented 11 years ago

@mbj @indrekj fair enough :)

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling 07865eedb804aa8eed0bd1a9e2b2863ceb45238e on indrekj:customizable-rules into c0113cb7611fcecb8cdfc3ce05380da92856e526 on dkubb:master.

dkubb commented 11 years ago

I agree with this. Start with strict defaults, but allow people to disable things selectively (or keep something enabled for new code, but specify exceptions for older code you either plan to clean up later or ignore). If we want yardstick to be used outside of our ecosystem of ultra-strict gems we need to offer some configurability ;)

I'll look over the code shortly and provide comments on specifics. I did notice though that the build is broken for 1.8.7 and ree. It looks like it could be reek pointing out some issues in the new code.

dkubb commented 11 years ago

@indrekj I just did a first pass on this code and wow, this is really good!

Please don't get discouraged by the amount of comments in the code. It's a sign I care ;) ... I'm most likely to provide the most feedback when someone put a lot of effort in the code. Just ask @snusnu, @mbj or @solnic ..

Also thank you for organizing the new specs using the mutant format. Yardstick is actually the first gem I ever wrote using what would eventually "DM2 style", and things evolved a bit after people started contributing and writing their own gems. I plan on updating Yardstick to work with devtools shortly, and having some of the specs arranged this way will go a long way towards getting things in order.

solnic commented 11 years ago

oh wow this is so good :smile:

indrekj commented 11 years ago

@dkubb Thank you for your feedback. I'll start improving the code in the evening or at the weekend.

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling 8918056dbd3465628bdd031480de5c1d08b2d525 on indrekj:customizable-rules into c0113cb7611fcecb8cdfc3ce05380da92856e526 on dkubb:master.

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling e2380a017934a54b5d895e8e9554aaf59dee6990 on indrekj:customizable-rules into c0113cb7611fcecb8cdfc3ce05380da92856e526 on dkubb:master.

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling aec65e438f06fbe54a407a2e99ec9fb2c1de3b7e on indrekj:customizable-rules into c0113cb7611fcecb8cdfc3ce05380da92856e526 on dkubb:master.

coveralls commented 11 years ago

Coverage Status

Coverage remained the same when pulling 8a055bdc22572b925b7ae06bccac3d726e7aa87b on indrekj:customizable-rules into c0113cb7611fcecb8cdfc3ce05380da92856e526 on dkubb:master.

indrekj commented 11 years ago

Are there any other things I should change?

I'd also like to use devtools/rpsec2 so I could do some testing with mutant, but I think introducing devtools should not be part of this PR.

Also, I'm not sure what should I do with the def_delegators code.

dkubb commented 11 years ago

@indrekj This is pretty awesome. I'm going to merge it now.

I left a couple of tiny comments, but it's nothing that should prevent merging. I can grant you commit access to the project so that if you want to make changes directly you can. For larger changes, like say adding devtools, you'll probably still want to branch and create a PR just for the review/discussion.

indrekj commented 11 years ago

@dkubb Thanks. I probably won't make any commits directly. I don't trust myself very much yet :smile:.

I'll try to bring devtools to yardstick in my next PR. Probably starting in the next week.