Skip to content

(PF-309) Make puppet module build use metadata.json as primary input#2147

Closed
thallgren wants to merge 1 commit intopuppetlabs:masterfrom
thallgren:PF-309
Closed

(PF-309) Make puppet module build use metadata.json as primary input#2147
thallgren wants to merge 1 commit intopuppetlabs:masterfrom
thallgren:PF-309

Conversation

@thallgren
Copy link
Contributor

The Forge team has decided that the current way of building, using the
Modulefile as the source for metadata, should change. The Modulefile
will become obsolete and the metadata.json will take its place as the
primary source. This means that the tool should no longer make an
attempt to generate the metadata.json.

This commit changes the current behavior so that when a metadata.json
is discovered, then it will be used verbatim. No types or providers
will be added to it.

The metadata.json will still be generated in case it is missing and
the Modulefile is present. A warning will be emitted to notify the
user that the Modulefile is deprecated.

The checksums that used to end up in the generated metadata.json will
now always be emitted to a separate checksums.json file.

When both the Modulefile and the metadata.json are present, the former
is ignored and a warning is emitted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wordsmithing: "Modulefile-based builds are deprecated ([link-to-docs?]); using metadata.json instead."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are Cloudsmiths > Wordsmiths?

Regarding the link to docs, I would expect a link to the module publishing doc for the relevant Puppet version that contains all the info about module metadata. I think that's http://docs.puppetlabs.com/puppet/3/reference/modules_publishing.html but @laurenrother is the authority here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to see a shortened link to information about the deprecation inlined… That's personal preference, however, and I'd love @laurenrother to make a pass over the words used for both of these warnings.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally opposed to links like that. They're too likely to become stale.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use http://links.puppetlabs.com/ to create a link / short link and make it refer to whatever is the most recent documentation. Hardcoding links without indirection is not good. (This scheme is used for deprecations in
puppet core). (Most recent example is this: See http://links.puppetlabs.com/puppet-mutation-deprecation - which I still had in my copy-paste buffer :-) ).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per conversation with Ryan and Alex, we're going to skip linking on this go-round. I am unwilling to link to even latest/reference/modules_publishing, just in case the site ends up being rearranged. If any link were to be present, I would prefer to mimic the Puppet team and link to a public issue.

Wording:
Modulefile is deprecated. Using metadata.json.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laurenrother using links.puppetlabs.com allows us to change the destination of the link later. We use it exactly for the reason that we might need to change what the deprecation message references over time.

@puppetcla
Copy link

CLA signed by all contributors.

@adrienthebo
Copy link
Contributor

@thallgren @pvande can we get an update on this pull request? Should it be left open or can we close it for now?

@thallgren
Copy link
Contributor Author

@adrienthebo, I just rebased and updated this pull request with new functionality (generation of the checksums.json)) so I'd prefer if it could remain open, be reviewed, and hopefully accepted.

@zaphod42
Copy link
Contributor

@thallgren @pvande since this touches the module tool, I'll leave it to you all on the forge team to determine if this is good and merge it in. One thing that does appear to be missing right now, though, is that it doesn't reference a puppet ticket in jira.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation seems wrong here

The Forge team decided that the current way of building, using the
Modulefile as the source for metadata, should change. The Modulefile
will become obsolete and the metadata.json will take its place as the
primary source. This means that the tool should no longer make an
attempt to generate the metadata.json unless it's missing.

This commit changes the current behavior so that when a metadata.json
is discovered, then it will be used verbatim. No types or providers
will be added to it.

The metadata.json will still be generated in case it is missing and
the Modulefile is present. A warning will be emitted to notify the
user that the Modulefile is deprecated.

The checksums that used to end up in the generated metadata.json will
now always be emitted to a separate checksums.json file.

When both the Modulefile and the metadata.json are present, the former
is ignored and a warning is emitted.
@thallgren
Copy link
Contributor Author

Corrected indentation and added related ticket to commit comment.

@adreyer adreyer closed this Jan 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants