Roles rules

[jaymzh]

How do you feel about implementing roles rules?

I'm about to write an SVN pre-commit that enforces the name of the role file is also the name of the role, which is easy to do outside of FC, but since you can define attributes and stuff in roles, it may be cool to have the FC framework for them.

[acrmp]

Hi Phil,

Sounds useful. To flesh this out a bit more:

• Add a new option -R to specify the role path. Accepts a directory or file path.
• Add role to the rule dsl.
• Do you want to be able to call foodcritic with a role path but without a cookbook path?

Rules that could be added using this:

• Has a run_list been defined (either default or in env_run_lists)?
• Can the roles and recipes in the run list be resolved?

For now I'd leave out:

• Support for roles expressed as json
• Inferring the role path from the cookbook path

Thanks,

Andrew.

[jaymzh]

Yeah. I'd need to be able to call it without a cookbook - we run FC as a svn pre-commit, so if no cookbook was changed in the precommit, there's nothing to check. Which begs the question, perhaps FC should take -C and -R, and, optionally, at some point in the future perhaps drop the "arguments are cookbook paths" behavior?

Both of those checks sound awesome to me.

I agree there's no reason to support JSON roles since those are deprecated anyway... and yeah, we shouldn't infer paths.

[acrmp]

-C is already reserved for --context at present. So is -r for REPL so it gets messy quickly.

We could take a type flag and keep the same foodcritic [path] interface.

These would be equivalent:

• foodcritic -T role my_role_path
• foodcritic --lint-type role my_role_path

These would be equivalent:

• foodcritic my_cookbook_path
• foodcritic -T cookbook my_cookbook_path
• foodcritic --lint-type cookbook my_cookbook_path

This would mean that you couldn't check both cookbooks and roles in one command.

[jaymzh]

how about -c for cookbooks and -R for roles? or -B for cookBooks?

I don't like the type option... that makes me sad.

[acrmp]

Can you elaborate on why?

I prefer type to mixed case which would annoy me because I'd expect the option case to be consistent and guess incorrectly. Type means we could also extend linting to environments or other stuff without an exploding list of cmd options.

[jaymzh]

Because (1) My primary use-case is a precomit where I want to be able to lint everything in one pass and (2) I don't want to have to run FC twice for myself or make other people do it either.

If you want consistent cases then -R for roles and -B for cookbooks?

[acrmp]

I think you're right that the ability to lint everything in one go trumps misgivings about the number of arguments.

So just to summarise for clarity:

These would be equivalent:

• foodcritic my_cookbook_path
• foodcritic -B my_cookbook_path
• foodcritic --cookbook-path my_cookbook_path

These would be equivalent:

• foodcritic -R my_role_path
• foodcritic --role-path my_role_path

Leaving us open for:

• foodcritic -E my_environment_path
• foodcritic --environment-path my_environment_path

Ok?

[acrmp]

And these would all be valid:

foodcritic -B my_cookbook_path -R my_role_path
foodcritic -R my_role_path my_cookbook_path
foodcritic -R my_role_path -B my_cookbook_path -B my_other_cookbook_path
foodcritic -B my_cookbook_path

[jaymzh]

Perfect!

[acrmp]

Awesome. I'll look to implement this soon.

Thank you for contributing your ideas and making foodcritic better - keep 'em coming!

[jaymzh]

Is the filename already available to rules? If not, I think it'll be particularly useful for roles and environments...

[acrmp]

The filename is passed as the second argument to the block:

recipe do |ast,filename|
  # ...
end

In the case of the cookbook block there is only a single argument (the cookbook directory).

[acrmp]

Sorry for the delay - I haven't forgotten about this.

[jaymzh]

Any updates?

[jaymzh]

Ping?

[acrmp]

Hi Phil,

This was released in foodcritic 3.0.0.

Thanks,

Andrew.