> > == Section 10.7 Defines can't use inherits for parameter defaults >> Should this be reduced to only cover classes, or should the description >> be expanded to cover the style of setting defined resource defaults also? >> (Which happen to be the "bad" example; whoops.) >> > > > The guide's preferred form works for defined types, too, provided the > default values are static (but then why would you even consider the other > style?). On the other hand, the guide's preferred style does not work even > for classes when a parameter's default value needs to be computed from the > actual values of other parameters. I think the guideline should be > softened to a recommendation, its applicability narrowed to classes only, > and the example labels changed from "GOOD" and "BAD" to something like > "Preferred" and "Discouraged". The non-applicability to defined types > should be explicitly called out, for clarity. > > Okay.
>> == Section 11.2 Recommendation against using "include" is unclear or >> harmful >> I think this was to describe how an over-zealous use of `include` may >> cause classes to be declared with the default parameters before a user may >> declare them with parameters. Or it could have been about cross-module >> class inclusion in public modules. What should we do with this section? >> >> > > I don't think there is any over-zealousness problem associated with using > 'include', unless it is with respect to declaring classes that are not > directly required in the scope of the declaration. Needed classes > *should* be declared, and that generally should be done via an 'include', > 'require', or 'contain' call. (And note, too, that 'contain' and 'require' > have all the same declaration semantics as 'include', but the current text > ignores them.) > > In fact, I think the guide is almost exactly backward on this matter. The > style points that should be recommended are: > > - Classes and defined types should declare the classes on which they > rely > - Data should generally be bound to classes via Puppet's Hiera-based > automatic data-binding facility, instead of via resource-style class > declarations. In particular, > - In modules, the resource style should *never* be be used to declare > *public* classes of that or any other module. > - Everywhere, the 'include', 'require', and / or 'contain' > functions should be used instead of resource-style class declarations > if at > all possible. > > All that can be well justified based on Puppet's design and > implementation, though I grant that "if at all possible" is pretty > wishy-washy. I'd go further with a style guide for my own site, but the > things I'd add would be a bit more controversial. > > Well put :). Re-reading this section makes me think that it was historically about the "declare variables then include classes" pattern that parameterized classes were created to obviate, but got mashed together with a design pattern for cross-module class inclusion like in a game of telephone. The whole "non-deterministic scoping" thing is probably irrelevant at this point. > == Section 10.4 Chaining arrow syntax with only references, not >> declarations >> Agreed. And perhaps that if there are line breaks around arrow syntax, >> they must only happen to the right of the arrow and not the left so that >> arrows are not "hidden" at the end of previous lines. >> > > > I'm missing it: how does allowing line breaks only to the *right* of a > chain operator prevent it from being hidden at the end of a line? > Shouldn't that be "left"? > Whoops yes, to the left of the arrow. Perhaps I was thinking "with the arrow to the right of the linebreak" when I wrote that. IE, https://github.com/puppetlabs/puppetlabs-mysql/blob/3.1.0/manifests/server.pp#L64-L72 is a bad example as the linebreaks are on the right of the arrow :). But anyway... V > > Chaining resource declarations doesn't actually bother me, personally. I > won't particularly resist the style guide discouraging that, but I'd be at > least as well satisfied by a softer guideline. For example, "a chain > operator should appear on the same line as its right-hand operand." Or the > still-softer "a chain operator should appear either on the same line as its > right-hand operand or on a line by itself." > I like this more than talking about left/right linebreaks. == Section 12.1 $unique_name = $name is unclear >> I believe this was to describe how the continued use of $name throughout >> a define can lead to confusion, as $name has no strong semantic meaning. >> Thus a "good" example would be https://github.com/puppetlabs/ >> puppetlabs-apache/blob/1.2.0/manifests/listen.pp#L2 and a bad example >> would be... https://github.com/puppetlabs/puppetlabs-apache/blob/1.2.0/ >> manifests/vhost.pp (because $name is scattered throughout the define and >> has no definite meaning). >> >> > > I don't think "unique_name" has any more or less clear of a meaning in > such contexts than does "name". Any lack of clarity is about which entity > the name belongs to, and describing it as "unique" doesn't really help with > that. > > Moreover, I observe that what you're saying the section is about is not at > all how I read it. (Lack of clarity? Check.) I don't think we can talk > about the wording details until we settle more generally what the section > is trying to say. If you're confident about that (you don't sound it) then > we can discuss the wording, but at this point my position would be that the > section should just be deleted. > I do know what I want, but I'm not a great wordsmith :(. The point of the section was to say that "$name in a define doesn't have a semantic meaning, so if you're going to use $name for anything programatic then assign it to a semantically-meaningful variable for clarity's sake and use that instead." But if you think that's not really a useful pattern then we could delete the section. I guess the best I can do is give the two examples linked above. If you want better words for it, I'll have to defer to Lauren. > == "Style guide" vs "specification" discussion from jcbollinger >> To paraphrase, "the 'language style guide' is written as a technical >> specification covering more than the language; it covers module structure >> and module contribution conduct. This is not 'style' and is beyond the >> scope of this document." >> >> In answer... we should change the title to something other than "The >> Puppet Language Style Guide" :). I personally feel that "Module Style >> Guide" would still encompass what is included here, when the above edits >> are taken into account. >> >> > > A *bona fide* style guide is appropriate and wanted. Moreover, it is > inappropriate to present a document that purports to be a general > specification for how *all* modules *must* be implemented, when in fact > there are many viable alternatives. PL has a lot of standing in this area, > but not so much as to lay down hard general rules like that. > > Instead of recasting the current document as something else, it should be > split into at least two documents. One would be a true style guide (or > even a "manual") describing PuppetLabs's official style for DSL code. Key > there would be that any "must", "must not", or equivalent in it be > explicitly in the context of "in order to comply with the style described > in this document". Another document might be a module design and layout > specification, perhaps for enforcement on modules published on the Forge. > This second document might require the modules to which it pertains to > comply with the style described in the separate style guide, so as to have > the effect (in its scope) of a combined document. > Thanks for the clarification. We'll take this into account for the next major revision. -- You received this message because you are subscribed to the Google Groups "Puppet Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/CAJaQvGAMuOrMVMmL9nJLqGMGu8-_YqBQpBSc12MRB96vAxaH1Q%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
