> > == 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 [email protected]. 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.
