Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(maint) fix desc/docs confusion #141

Merged
merged 2 commits into from
Jun 19, 2019
Merged

Conversation

DavidS
Copy link
Contributor

@DavidS DavidS commented May 29, 2019

The text and examples have been inconsistent with how desc vs docs
has been handled. This change fixes the text and examples to all show
and require desc, but accept docs in places where we showed it
previously.

@DavidS DavidS requested review from da-ar and clairecadman and removed request for da-ar May 29, 2019 14:33
DavidS added a commit to DavidS/puppet-resource_api that referenced this pull request May 29, 2019
This implements the changes specified in puppetlabs/puppet-specifications#141

> The text and examples have been inconsistent with how `desc` vs `docs`
> has been handled. This change fixes the text and examples to all show
> and require `desc`, but accept `docs` in places where we showed it
> previously.
@@ -56,7 +56,7 @@ Puppet::ResourceApi.register_type(
The `Puppet::ResourceApi.register_type(options)` function takes the following keyword arguments:

* `name`: the name of the resource type.
* `desc`: a doc string that describes the overall working of the resource type, provides examples, and explains prerequisites and known issues.
* `desc`: a doc string that describes the overall working of the resource type, provides examples, and explains prerequisites and known issues. Can also be written as `docs` for backwards compatibility. This compatibility option will go away in the next major revision.
Copy link
Contributor

Choose a reason for hiding this comment

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

The term "next major revision" does not work so well since it is a reference to the resource-api next major version as opposed to a revision of the specification. Maybe say "in a future major revision"?

Copy link
Contributor

Choose a reason for hiding this comment

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

hm.. or is this meant to be a "revision of the spec" - in which case we should perhaps formalize the versions of the specifications - since the entire repo covers "all the things"...

Copy link
Contributor Author

@DavidS DavidS May 29, 2019

Choose a reason for hiding this comment

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

good point. I'll clarify to express that I mean the gem.

raises the wider question of whether this is still a good place to keep this document or if it were better to put this spec into the https://github.com/puppetlabs/puppet-resource_api repo and synchronize it there with the different release train branches.

Copy link
Contributor

Choose a reason for hiding this comment

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

It makes sense for it to be here if there can be alternative implementations of the API, but must then be written to be free of implementation concerns. For example, this could be written as "an implementation may support the older docs tag to have the same meaning as desc. And add a note that PDK version such and such supports, and is expected to drop in...

This could be pedantic and somewhat silly when there is only one project implementing the specification.
Just my 2c.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is only one implementation and any compatible other implementation would need to implement this fallback too to be compatible. I'll leave it as it is for now.

The text and examples have been inconsistent with how `desc` vs `docs`
has been handled. This change fixes the text and examples to all show
and require `desc`, but accept `docs` in places where we showed it
previously.
DavidS added a commit to DavidS/puppet-resource_api that referenced this pull request May 29, 2019
This implements the changes specified in puppetlabs/puppet-specifications#141

> The text and examples have been inconsistent with how `desc` vs `docs`
> has been handled. This change fixes the text and examples to all show
> and require `desc`, but accept `docs` in places where we showed it
> previously.
Co-Authored-By: clairecadman <claire.cadman@puppet.com>
@@ -56,7 +56,7 @@ Puppet::ResourceApi.register_type(
The `Puppet::ResourceApi.register_type(options)` function takes the following keyword arguments:

* `name`: the name of the resource type.
* `desc`: a doc string that describes the overall working of the resource type, provides examples, and explains prerequisites and known issues.
* `desc`: a doc string that describes the overall working of the resource type, provides examples, and explains prerequisites and known issues. You can also write `desc` as `docs` for backwards compatibility. This compatibility option will no longer be available in the next major revision of the library.
Copy link
Contributor

Choose a reason for hiding this comment

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

Will you change this text once the "next major revision of the library" is released?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I've been pretty good until now to keep this text and the library in sync. I expect to do so in the foreseeable future, too.

DavidS added a commit to DavidS/puppet-resource_api that referenced this pull request May 30, 2019
This implements the changes specified in puppetlabs/puppet-specifications#141

> The text and examples have been inconsistent with how `desc` vs `docs`
> has been handled. This change fixes the text and examples to all show
> and require `desc`, but accept `docs` in places where we showed it
> previously.

# Conflicts:
#	lib/puppet/resource_api.rb
#	lib/puppet/resource_api/transport.rb
#	lib/puppet/resource_api/type_definition.rb
#	spec/puppet/resource_api/base_context_spec.rb
#	spec/puppet/resource_api/base_type_definition_spec.rb
#	spec/puppet/resource_api/puppet_context_spec.rb
#	spec/puppet/resource_api/transport_spec.rb
DavidS added a commit to DavidS/puppet-resource_api that referenced this pull request May 30, 2019
This implements the changes specified in puppetlabs/puppet-specifications#141

> The text and examples have been inconsistent with how `desc` vs `docs`
> has been handled. This change fixes the text and examples to all show
> and require `desc`, but accept `docs` in places where we showed it
> previously.
DavidS added a commit to DavidS/puppet-resource_api that referenced this pull request Jun 12, 2019
This implements the changes specified in puppetlabs/puppet-specifications#141

> The text and examples have been inconsistent with how `desc` vs `docs`
> has been handled. This change fixes the text and examples to all show
> and require `desc`, but accept `docs` in places where we showed it
> previously.
DavidS added a commit to DavidS/puppet-resource_api that referenced this pull request Jun 12, 2019
This implements the changes specified in puppetlabs/puppet-specifications#141

> The text and examples have been inconsistent with how `desc` vs `docs`
> has been handled. This change fixes the text and examples to all show
> and require `desc`, but accept `docs` in places where we showed it
> previously.
@DavidS
Copy link
Contributor Author

DavidS commented Jun 19, 2019

Merging this as the implementation is now merged.

@DavidS DavidS merged commit 6bd1811 into puppetlabs:master Jun 19, 2019
@DavidS DavidS deleted the fix-desc-docs branch June 19, 2019 10:53
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.

None yet

3 participants