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

Correct formatting of deprecation notices. #340

Merged
merged 1 commit into from
Aug 29, 2021
Merged

Correct formatting of deprecation notices. #340

merged 1 commit into from
Aug 29, 2021

Conversation

dsymonds
Copy link
Contributor

There is a specific format for deprecation notices in Go doc comments
(see https://golang.org/wiki/Deprecated) that ensures they are correctly
rendered (e.g.
https://pkg.go.dev/github.com/PagerDuty/go-pagerduty#Client.CreateIncidentNote).

Add a handful of missing deprecation notices to functions that now have
parallel WithContext counterparts.

@theckman
Copy link
Collaborator

I was sorta reluctant to mark these as deprecated in this way, as the methods aren't going away in v2. Instead, the WithContext ones will be renamed effectively changing the parameters of the current ones.

@dsymonds
Copy link
Contributor Author

Well, some are already marked as deprecated, just inconsistently, and even until v2 it seems good to steer people away from the non-context versions. More importantly, even when v2 comes along, v1 will be around for a long time, so if you're using it you need to know about this.

@theckman theckman added this to the v1.5.0 milestone Jul 7, 2021
Copy link
Collaborator

@theckman theckman left a comment

Choose a reason for hiding this comment

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

Looking to get this in as part of the v1.5.0 release, but I think the comments need to be updated based on the context in the wiki. I've added a single comment in my review, but it applies to all of the proposed changes I believe.

ability.go Show resolved Hide resolved
There is a specific format for deprecation notices in Go doc comments
(see https://golang.org/wiki/Deprecated) that ensures they are correctly
rendered (e.g.
https://pkg.go.dev/github.com/PagerDuty/go-pagerduty#Client.CreateIncidentNote).

Add a handful of missing deprecation notices to functions that now have
parallel WithContext counterparts.
Copy link
Collaborator

@theckman theckman left a comment

Choose a reason for hiding this comment

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

I'll merge next time I'm in front of my workstation. Thank you!

@theckman theckman merged commit d0d5cb1 into PagerDuty:master Aug 29, 2021
@theckman
Copy link
Collaborator

Finally got my pending PR approved by a PD employee and so I could merge this. Thanks again for the contribution!

@dsymonds dsymonds deleted the deprecate-more branch August 29, 2021 22:13
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

2 participants