Style guide question..

[Brian Gupta]

According to the style guide, "Classes and defined resource types
must not be defined within other classes."

However looking at
https://github.com/puppetlabs/puppetlabs-nginx/blob/master/manifests/init.pp
shows that there are a number of other classes defined in class nginx.

What's the correct pattern? (And why?)

Also can someone confirm the following class naming standards.:
1) In init.pp, naming the main class associated with that module as
the same name as that module is best practice and will get applied,
when someone just does include modulename.
2) For all classes that you want to include from other modules you
need to name them modulename::classname
3) Other then option 1, one should avoid just using a simple classname
inside a module, but rather use modulename::classname. (Some
additional clarification here would be great.)

Thanks,
Brian

[David Schmitt]

These recommendations are rooted in the autoloader, who automatically
finds modulename::some::class in modulename/manifests/some/class.pp.

Additionally this improves orientation for others when reading or using
the module because it ibues the names with meaning.


Best Regards, David

[Brian Gupta]

Thanks David. Could you clarify one thing?

The style guide says the following is bad:

" class foo {
...
class bar { ... }
}"

I think this largely works, and will register as classes apache and
apache::ssl, but the correct style would be?:

class foo {
---
}
class foo::bar {
...
}

>
>
> Best Regards, David
>
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Users" group.
> To post to this group, send email to puppet...@googlegroups.com.
> To unsubscribe from this group, send email to
> puppet-users...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/puppet-users?hl=en.
>

[David Schmitt]

Yes. Especially when you put foo into foo module's init.pp and foo::bar
into bar.pp of the same module.


D.

[jcbollinger]

On Friday, June 29, 2012 1:51:52 AM UTC-5, Brian Gupta wrote:

According to the style guide,  "Classes and defined resource types
must not be defined within other classes."

However looking at
https://github.com/puppetlabs/puppetlabs-nginx/blob/master/manifests/init.pp
shows that there are a number of other classes defined in class nginx.

You are misreading that manifest: it does not define any classes in class nginx.  Rather it declares several classes using parameterized class syntax.  "Defining" a class is this:

class foo {
  # declare resources
}

"Declaring" a class is this:

class { 'foo':
  # parameters, if any
}

or

include "foo"

or

require "foo"

The manifest you referred to does only the latter.  I'd argue that it could and should use "include" for those instead of parametrized class syntax, but it does not conflict with the style guide point you referenced.

Note, however, that elsewhere the style guide (version 1.1.2) says "Classes should generally not declare other classes," which is a bunch of bologna.  A Puppetlabs employee told me recently that he would have that removed, but evidently it hasn't happened yet.

As far as I am concerned, classes absolutely should declare other classes wherever they have parse-time dependencies on them, and it's fine for classes to declare other classes to effect composition.  That's no big deal if you use include/require to declare classes, because classes can be declared any number of times, anywhere, if you use only those mechanisms.  It runs into trouble if you declare classes using the parametrized-class syntax, however, as a declaration of that kind produces an error if the class has already been declared.  That has been my biggest objection to parametrized classes ever since they were introduced.

The style guide's provision makes some sense in the context of a site driven by a comprehensive ENC responsible for declaring every needed class at top level, in a suitable order.  Use of an ENC in such a way is uncommon and unwise, however, and it doesn't play nicely with third-party modules.  It's ridiculous to make that a general provision for good style.  It does guard against needing to pass parameters down through multiple layers of classes to get them to where they're actually needed, but if you avoid parametrized class syntax (as you should) then that's a non-issue.
 

What's the correct pattern? (And why?)

Each class and defined type should be defined in its own file, named and located according to the autoloader's expectations.  Note that this does not prevent using nested namespaces (one of the things you get from lexically nesting class and defined type definitions inside classes), and it does not prevent classes and defined types from referring to class variables of other classes (though they must use qualified names to do so).

Generally speaking, every class should declare all classes on which it has compile-time dependencies, to the extent that it can do so.  Such dependencies come mostly from referencing variables of other classes or resources declared by other classes.

Classes may declare other classes on which they do not otherwise have compile-time dependencies, especially other classes from the same module, to form pre-composed units that are convenient for users to declare.  It is reasonable and often appropriate to design modules around this strategy.

Parametrized class declaration syntax should be avoided at all costs.  In Puppet 2, that largely neuters parametrized classes (use external data instead).  In Puppet 3 you can provide class parameter values via external data where default values do not suit (so you're pretty much still using external data instead, but with a convenient interface).
 

Also can someone confirm the following class naming standards.:
1) In init.pp, naming the main class associated with that module as
the same name as that module is best practice and will get applied,
when someone just does include modulename.

Yes.  Note also that it is optional to have a main class for the module (that you can declare via "include 'modulename'") at all.

 

2) For all classes that you want to include from other modules you
need to name them modulename::classname

All classes should be named (namespace::)+classname (e.g. mymodule, mymodule::myclass, mymodule::sub_namespace::myclass).  Module main classes, too, based on the convention that they appear in the top-level namespace.  Modules establish namespaces for other classes within them, and additional namespaces can be created within those (but don't do that by nesting classes or definitions inside class definitions).
 

3) Other then option 1, one should avoid just using a simple classname
inside a module, but rather use modulename::classname. (Some
additional clarification here would be great.)

I would simply say that all classes should be referenced by their fully-qualified names everywhere.  There is no need to distinguish option 1 here, as those classes simple names are also their fully-qualified names.


John

[David Schmitt]

On 29.06.2012 15:55, jcbollinger wrote:
>
>
> On Friday, June 29, 2012 1:51:52 AM UTC-5, Brian Gupta wrote:
>
> According to the style guide, "Classes and defined resource types
> must not be defined within other classes."
>
> However looking at
> https://github.com/puppetlabs/puppetlabs-nginx/blob/master/manifests/init.pp
> <https://github.com/puppetlabs/puppetlabs-nginx/blob/master/manifests/init.pp>
>
> shows that there are a number of other classes defined in class nginx.
>
>
>
> You are misreading that manifest: it does not define any classes in

> class nginx. Rather it /declares/ several classes using parameterized

> class syntax. "Defining" a class is this:
>
> class foo {
> # declare resources
> }
>
> "Declaring" a class is this:
>
> class { 'foo':
> # parameters, if any
> }
>
> or
>
> include "foo"
>
> or
>
> require "foo"
>
> The manifest you referred to does only the latter. I'd argue that it
> could and should use "include" for those instead of parametrized class
> syntax, but it does not conflict with the style guide point you referenced.
>
> Note, however, that elsewhere the style guide (version 1.1.2) says

> "*Classes should generally not declare other classes*," which is a bunch

> of bologna. A Puppetlabs employee told me recently that he would have
> that removed, but evidently it hasn't happened yet.

Indeed! Even more so as using the word "declare" for "include" is a load
of utter bollocks. "Declare" in the common programming language sense
means "indicating the existence of a thing that is defined elsewhere."
Like "declaring an external variable" or "(pre-)declaring a method", and
never indicates actions taken.


Best Regards, David

[jcbollinger]

On Friday, June 29, 2012 9:56:04 AM UTC-5, David Schmitt wrote:

Indeed! Even more so as using the word "declare" for "include" is a load
of utter bollocks. "Declare" in the common programming language sense
means "indicating the existence of a thing that is defined elsewhere."
Like "declaring an external variable" or "(pre-)declaring a method", and
never indicates actions taken.

 
You have a point there, but perhaps not the one you think you do.  Puppet's DSL is not a programming language, so applying linguistic conventions associated with programming languages is not entirely fair.  Inasmuch as the DSL is a declarative language, in fact, more or less everything is a declaration, or part of one.  Thus the problem with the term "class declaration" is not that it's inaccurate (it's a declarative statement that the target node has / belongs to the named class), but rather that it's too generic when everything else is a declaration too.

I used to prefer the term "include", but that doesn't fit well because it also has to cover the "require" function and the parametrized-class syntax.  I eventually gave in to what seems to be the prevailing terminology, at least on this group.  If you have an alternative that is better suited then I'm all ears.  Maybe we can start a trend with it :-)


John

[Eric Shamow]

On Friday, June 29, 2012 at 9:55 AM, jcbollinger wrote:

Note, however, that elsewhere the style guide (version 1.1.2) says "Classes should generally not declare other classes," which is a bunch of bologna.  A Puppetlabs employee told me recently that he would have that removed, but evidently it hasn't happened yet.

I'm wondering if it would be better to say "classes should generally not *define* other classes," which I find a lot more objectionable.

-Eric

[jcbollinger]

You mean "less objectionable" I presume?  I agree that classes generally should not define other classes, but the guide already says that elsewhere.  I guess there's no harm in repeating it, if you think it worth the emphasis.


John

[Eric Shamow]

On Tuesday, July 3, 2012 at 8:39 AM, jcbollinger wrote:

You mean "less objectionable" I presume?  I agree that classes generally should not define other classes, but the guide already says that elsewhere.  I guess there's no harm in repeating it, if you think it worth the emphasis.

Sorry, yes - I meant that the practice was more objectionable rather than the phrasing.

However if it's defined elsewhere no need to repeat.

-- 

Eric Shamow

Professional Services

http://puppetlabs.com/

(c)631.871.6441

 

[David Schmitt]

On 2012-06-29 23:27, jcbollinger wrote:
>
>
> On Friday, June 29, 2012 9:56:04 AM UTC-5, David Schmitt wrote:
>
> Indeed! Even more so as using the word "declare" for "include" is a
> load
> of utter bollocks. "Declare" in the common programming language sense
> means "indicating the existence of a thing that is defined elsewhere."
> Like "declaring an external variable" or "(pre-)declaring a method",
> and
> never indicates actions taken.
>
>
> You have a point there, but perhaps not the one you think you do.
> Puppet's DSL is not a programming language, so applying linguistic
> conventions associated with programming languages is not entirely fair.
> Inasmuch as the DSL is a declarative language, in fact, more or less

> /everything/ is a declaration, or part of one. Thus the problem with the

> term "class declaration" is not that it's inaccurate (it's a declarative
> statement that the target node has / belongs to the named class), but
> rather that it's too generic when everything else is a declaration too.

English is only my second language, but I still see the difference
between "declare a class" and "declare *membership* in a class". A
subtle difference that is violated thoroughly in the style guide.

> I used to prefer the term "include", but that doesn't fit well because
> it also has to cover the "require" function and the parametrized-class
> syntax. I eventually gave in to what seems to be the prevailing
> terminology, at least on this group. If you have an alternative that is
> better suited then I'm all ears. Maybe we can start a trend with it :-)

Includes, depends on, requires, delegates to, requests, inherits from,
pulls in, activates, requests, hauls in.

Hope dies last, as they say ;-)


Best Regards, David