-
Notifications
You must be signed in to change notification settings - Fork 582
Stdlib::Http::Method: Add new type for http methods #1192
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me but I'd drop the Puppet >= 4.5 part in the tests since it's not supported anyway.
This PR creates new new resources: * Stdlib::Http::Method for validating http methods * Stdlib::Http::Status This is just a copy of Stdlib::Httpstatus * make Stdlib::Httpstatus and alias to Stdlib::Http::Status Ideally we would deprecate Stdlib::Httpstatus in favour of Stdlib::Http::Status
b4ebf99
to
6c3aa83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also just realized this may generate additional things in REFERENCE.md
. Would you mind regenerating that and making sure the output makes sense?
@@ -1 +1 @@ | |||
type Stdlib::HttpStatus = Integer[100, 599] | |||
type Stdlib::HttpStatus = Stdlib::Http::Status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this type can be deprecated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would also like to deprecate this in favour of Stdlib::Http::Status
but not sure of the process for that. i regret not namespacing this when i first introduced it :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can always do this bit in a separate pr if that makes things easier?
We end up with two sections for http status
|
Test Failure unrelated; this was based on the #1193, which passed. |
Im not sure what happened to this but i don't see it in the history https://github.com/puppetlabs/puppetlabs-stdlib/commits/main/types :/ |
This PR creates new new resources:
Ideally we would deprecate Stdlib::Httpstatus in favour of
Stdlib::Http::Status