-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Contributor Docs - add guide on deprecations and breaking changes #27654
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.
LGTM 🌻
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.
LGTM with a couple minor comments!
|
||
This deprecated resources has been removed from the Azure Provider. | ||
``` | ||
**Note:** The Resource documentation does not need to be updated with a note since the provider will inform any users using the resource of the deprecation with a warning. |
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 it is worth throwing a note into the resource docs anyway?
I'm thinking of a use case where I'm writing out a resource and get it all written just to find out when running Terraform that there is a new one to use instead and I could have written it out the new way instead of the old.
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.
That's a fair point. I'll update this to require a note in the docs for deprecated resources/datasources.
Breaking schema changes can include: | ||
- Property renames | ||
- When properties become Required | ||
- When properties have Computed removed in some cases |
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.
Could we expand on what are some cases
?
"version": { | ||
Type: pluginsdk.TypeString, | ||
Optional: true, | ||
Default: 1, |
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.
Minor but I believe this wants to be 2 and the Default below wants to be 1 based on the wording of the example default going from 1 to 2
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.
Thanks @stephybun - LGTM 🐃
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
Added a guide on how to do deprecations and breaking changes in the provider using the major release feature flag.