Behaviour change: Change coding style from "prefer single quotes" to "always use double quotes" #1019
Replies: 17 comments
-
|
👍 Double quotes everywhere is my preference as well. My understanding is that the performance implications for most use cases are rounding errors at this point. |
Beta Was this translation helpful? Give feedback.
-
|
@jbondpdx What would be the process to get the Language Style Guide updated to reflect the quoting preference change proposed here? |
Beta Was this translation helpful? Give feedback.
-
|
@DavidS and @scotje This was discussed ad nauseum in 2015-2016, and at the modules team gathering in Belfast in 2016, we decided to leave the recommendation (and style guide) alone, at least in part due to the amount of work it would take to change this in existing code. You can enjoy the JIRA comments here: https://tickets.puppetlabs.com/browse/DOCUMENT-274 Fortunately, we didn't etch it in stone or anything! Traditionally, modules has "owned" style guide, so I recently handed the style guide off to @clairecadman . Of course, in addition to modules team, our team has a vested interest in the style guide, as do Language (Henrik and Thomas) and the community. I think the first move should be a style guide ticket (DOCUMENT project) assigned to Claire. It's my personal opinion that changing code style is kind of a big deal, and changes should be decided by a cross-team group representing the aforementioned stakeholders. That said, it's Claire's to drive! |
Beta Was this translation helpful? Give feedback.
-
|
@DavidS @scotje Yes please create a docs ticket and assign it to me if you want updates to the style guide. Thanks! |
Beta Was this translation helpful? Give feedback.
-
|
I wrote the original email that Jean captured in that ticket. I still fully stand by my original comment and 100% approve of this change ticket. |
Beta Was this translation helpful? Give feedback.
-
|
I completely disagree with this approach. I fix far too many problems created by people not realizing what character combinations do, which ones are meta characters, etc. Forcing everyone to use double quotes all the time demands that everyone be aware of all meta combinations, all interpolation rules, etc etc. Talk about cognitive load! That said, I do understand that some people feel otherwise. I'd much prefer that we allow either pattern in puppet-lint, and don't mark down either one used consistently. |
Beta Was this translation helpful? Give feedback.
-
|
So in my original email back in 2015 I said "Use double quotes unless you have a good reason not to" When my students struggled with windows backslashes and strings with double quotes themselves, I didn't stand over them with a big stick and force them to figure it out - I just said "well that's a good reason" and told them do whatever was easiest. My whole rationale behind this is that arguing over what type of quotes to use is a zero-sum game. It's a complete waste of productive energy that could have been used to put things inside those quotes - double or otherwise. |
Beta Was this translation helpful? Give feedback.
-
|
Perhaps the style guide recommendation should be something more along the lines of "pick a default style for quoting string literals and apply it consistently to your codebase". PDK will still end up having to have a default for new modules but we should also leverage the config subsystem to allow users to set their preferred quoting style system-wide, and have PDK read from the config value when applying the default template. Thoughts? |
Beta Was this translation helpful? Give feedback.
-
|
I would love that. Allow each side to configure it for their own modules as they see fit, and thus allow advocates for both sides to coexist without conflict. |
Beta Was this translation helpful? Give feedback.
-
|
Opened https://tickets.puppetlabs.com/browse/DOCUMENT-931 to discuss changes to the style guide as requested. |
Beta Was this translation helpful? Give feedback.
-
|
Given the strong and divergent opinions around this topic, I support @scotje's suggestion to step away from recommending a specific quoting style, and only provide a |
Beta Was this translation helpful? Give feedback.
-
|
Here is a draft of the update to the quoting section of the style guide, please comment on the Google doc if you have any feedback! https://docs.google.com/document/d/1oIzh4FAzwVOn7BIWEFDAGxHnucaxv0I18J9j4tsVLwo/edit?usp=sharing |
Beta Was this translation helpful? Give feedback.
-
|
I just want to comment my support for the Let the people decide option to score it being consistent in the codebase, but not enforced to be one over the other. A problem for one person is often only a corner-case for someone else, and thus users should be able to decide. Our internal style guide is to use single quotes except in cases where interpolation is explicitly required. I find this works well with modules and Hiera YAML. |
Beta Was this translation helpful? Give feedback.
-
|
I'm usually all in favor of "let the people decide", but now you have shifted the cognitive burden significantly to contributors of a variety of modules/projects. If, as a community, we are all using the same quoting rules then you must learn those rules once and learn how to apply them once. That burden may be high, but now if you let each group/developer pick their own not only do you need to learn those rules you now also have to learn what the rules are for a particular project and incur the mental overhead of having to switch during the transition between different projects with different rules and fight against muscle memory when typing. I understand the desire to lower the cognitive burden on someone new to Puppet, and if PDK wants to default to "ignore quotes" to ease that developer's introduction to Puppet, especially for that developer's own/private modules, then I think that would be okay. In the interest of choice, I think even letting force-single / force-double to be okay options as well. But please please please do not change the style guide or the Forge checks. There is a huge advantage in having the community and community contributions consistent as a whole. The amount of uniformity in code style lowers the bar significantly for first time contributors (and people learning from reading others' code). |
Beta Was this translation helpful? Give feedback.
-
|
@seanmil I think the cognitive burden side of varying preferred quoting styles can and should be resolved by automatic tooling. (e.g. There is always going to be some degree of cognitive burden associated with switching between various projects and we are still setting very specific guidelines about how to apply each of the two styles so the variation will be pretty narrow in scope. |
Beta Was this translation helpful? Give feedback.
-
|
Style guide was updated to address single vs double quote preferences in Puppet Code. |
Beta Was this translation helpful? Give feedback.
-
|
Remaining changes here would be to update puppet-lint checks/defaults as needed and potentially add a default to PDK's rubocop config. @rodjek what do you think the best course of action is for puppet-lint? |
Beta Was this translation helpful? Give feedback.
-
Rationale
From the early days of using rubocop I have inherited the default style of "prefer single quotes when possible". As it turns out, this is a unhelpful style as it adds additional cognitive burden to developers (especially the majority of our target user group, who are not full-time ruby developers) in having to understand two sets of quoting rules, and when to switch up from one to the other when modifying code.
Proposal
To avoid that burden, I propose switching to a "always use double quotes" style. This might require a bit more backslash-escaping when there is no "technical" need for it (because the string doesn't contain a variable reference, and could be simplified by using single quotes), but - as referenced in the rationale - it'll reduce the amount of detailed ruby knowledge required overall.
The puppet style guide also picked up that deficiency, and will need to be adjusted together with the default puppet-lint config.
Expected impact
On the ruby side, rubocop can automatically fix up the style change. It'll be a one-off churn in each module when updating to the new style.
Puppet-lint will require some work to enforce the new style, make it configurable, and implement a autofixer.
References
Style/StringLiteralsdefault todouble_quotesrubocop/rubocop#5306'vs"is 100% the hill I will die on 🙂"ruby -e "puts 'foo'.inspect"➡️"foo"Beta Was this translation helpful? Give feedback.
All reactions