-
Notifications
You must be signed in to change notification settings - Fork 153
Rule FC001 contrary to Opscode recommendation #1
Comments
Thanks for the quick feedback Joshua. Congrats on a successful summit. I put this out there despite the code being a bit nasty (which is a little ironic). I have a list of rules I'd like to implement. I really need to compare these against the best practices in your training material / docs and get some feedback from people on which ones would be really cool vs not very helpful. Does GitHub issues work for this? The suggestion @dysinger noted of checking resources for missing attributes would be a really useful addition and probably hit most peoples 80/20: I personally find symbols easier to scan, more ruby-like and for some reason less susceptible to typos but I follow your argument. I'll take this out for now until there is a mechanism for defining policies. |
To be honest, I find hard to believe that someone that comes to chef and is not a rubist cannot understand what a symbol is but can understand how a block works. Said that, I don't understand why Opscode recommendation is contrary to the language good practices. Symbols are good as hash keys because they are inmutable and they are allocated only once, which saves you from a hell of performance issues. Btw, symbols can contain dashes, you only need to know the language: :"this-is-a-symbol-full-of-dashes" |
I sympathise with the Opscode view because symbols are one more thing a user new to both Chef and Ruby has to understand. This is the internal dsl problem of how much of the wider language do you have to grok to be productive. Opscode have chosen to optimise for ease of learning Chef rather than writing idiomatic ruby (to start with). At the moment the people motivated enough to seek out this tool are likely to be plenty capable of getting to grips with symbols so arguably that would be a more sensible default. @jtimberman any more to add? |
I don't find anything good in tell users to use the languange's bad practices, but of course Opscode have chosen that, they want people to use Chef regardless they know anything about the language the recipes are written with. It reminds me when people learned rails without knowing anything about ruby, everything was sooo magic. It's Opscode decision to teach how they want to teach, but a language's bad practice shouldn't be ever a recommended practice to write any stuff with that language. But this is just my opinion, if I have to use foodcritic I can just ignore that rule. |
@calavera My observation is that it is mainly Ruby developers with a background in Rails that use symbols for hash keys. However, I have not seen it mentioned in any coding style or best practice documents. If this is such a bad style, where is the reference of the best practice? Ruby totally lets you do either, and there are libraries that introduce new classes to seamlessly handle either (like Mash, which Chef uses). I'm genuinely curious, since you seem accusatory that we're stupid in some way, even though the decision to teach a particular form is actually well thought out. In Chef, strings and symbols are equally valid to use for accessing attributes. You can also use the auto-vivified methods. These three uses will all refer to the same attributes: node['apache']['port']
node[:apache][:port]
node.apache.port That's totally fine from a functionality standpoint. However, one of these is decidedly language neutral, for all intents and purposes, and will look familiar to the ways that other languages reference hash keys. If I were writing a book on Chef (I'm not, though someone else is), in the Node Attributes chapter I would only mention accessing attributes with strings for the keys. I would probably not mention the other forms, and defer them to an appendix that covers them in more detail. Symbols make the most sense to people already familiar with Ruby for a couple reasons.
It's totally fine if you want to rock symbols for getting attribute values. If foodcritic can support looking up either symbols or strings and reporting inconsistency, that'd be great. I don't care what people use or prefer, paint your bikeshed any color you want. In opening this issue, I am providing my feedback to this project representing the color that Opscode chose (green, with yellow stripes). [0]: of course, Chef uses all three together, so the memory consumption point is probably moot. |
isn't this resolved? |
@jtimberman I don't think @calavera was in any way suggesting that you're somehow stupid for not liking the use of symbols. All I see is disagreement with your rationale for discouraging symbols. I happen to agree: I don't see much benefit to preferring strings, and don't agree with the arguments you give. That doesn't mean I think less of you or your intelligence. I'd happily enjoy a beer with you (if we were in the same town) and discuss any number of things--either things we agree on or things we disagree on. Disagreement does not need to result in or be interpreted as hostility or disdain. I hope we can avoid falling into those traps. |
Speaking as a guy who has done some Chef but still really doesn't understand Ruby, and COMPLETELY fails to get the connection between blocks and symbols, I prefer the string method. KISS FTW. |
:'a-symbol-can-have-dashes' |
While we're not the authoritative source of how you should write cookbooks, we do teach (in our training materials, classes, and documentation) that node attributes should be accessed by quoted string literals rather than Ruby symbols, because it is the principle of least surprise, and will always work, as a key name as a string can contain dashes (while symbols cannot). Not everyone coming to, and using Chef is a Rubyist, and the symbol syntax may be surprising or inconsistent with other languages that use hashes.
https://github.com/acrmp/foodcritic/blob/master/lib/foodcritic/rules.rb#L1-2
The text was updated successfully, but these errors were encountered: