New Check: all ruby files should have an encoding marker #15

Closed
wants to merge 2 commits into
from

2 participants

@yob

I have a project where team style dictates that every ruby file should have an encoding marker.

This is admittedly not something every team wants and wouldn't be a good default check.

Implementing the check was straight forward ( cf7e549 ), however I was surprised at the size of the subsequent commit that wired it into cane ( ba85d41 ).

What do you think about an addition to the rake task that allows teams to load custom checks? Something like:

 Cane::RakeTask.new(:cane) do |cane|
   cane.use EncodingCheck.new
   cane.use MethodLengthCheck.new(15)
   cane.use ObscureTeamCheck.new
   cane.use FilteredParamsCheck.new
 end

end

@xaviershay

sorry I try to keep my code bases vegan :P

@xaviershay

The idea of the run method was to provide a place to default everything to off. Could --no-coding be moved there?

Actually I think I'd want --no-encoding to be the default behaviour, since AFAIK not many teams enforce this style.

@xaviershay

Yeah you're right, wiring in new things is kind of insane. Maybe there is a way to combine your use suggestion (which I like) with a way to also set up the right command line params, documentation, etc...

@yob

I'm not seriously proposing EncodingCheck for merging upstream, it was mostly to provide code and context for my 'use' suggestion.

The one problem with it is how to support the command line. Is that what you're referring to with "a way to also set up the right command line params, documentation"?

@xaviershay

Yeah, I was thinking something like:

class EncodingCheck
  def self.command_line_options
    # Returns some meta data sufficient to specify command line options and rake task
    {}
  end
end

Then the CLI class can just iterate through all known checks and pull the meta data off them. That way, implementing a new check is all confined to the one class, rather than having to sprinkle things throughout the code base. Maybe it's simpler not even to go the meta data route, instead just call EncodingCheck.translate_cli_options(options). I'd have to play with it.

@xaviershay

No obvious next action, closing.

@xaviershay xaviershay closed this Aug 1, 2012
@xaviershay

.use method is supported in 2.1.0

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