profiler: deprecate PprofDiff #1806
Merged
+7
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The PprofDiff function was added to our public API unintentionally as part of #1511. The function is only meant for internal use. This was missed during code review, likely due to the size of the change and the fact that most of our attention was devoted to making sure the core parts of the change were implemented correctly. This is an exception to our normal v1 compatibility guarantees. It is very unlikely that any of our users depend on this function. It serves no purpose in the context of the profiler's public API, and anybody who might have a reason to use this function would probably have already found a way to do what they want using the github.com/google/pprof/profile package.
|
I've marked this as a draft, so we don't merge it right away. I personally think this is okay to do. However, we could mark this as deprecated instead, with a note that this was not intended to be part of our API. I know pkg.go.dev understands deprecation warnings, and I think some Go IDEs will also show you if you're using a deprecated function. Then we could remove it in a later release. That could be more cautious than necessary, though. Thoughts? |
felixge
previously approved these changes
Mar 15, 2023
Instead of making it private right away, make it deprecated instead, to be removed in a later release. This should hopefully give users a chance to notice that it's going away and stop using it before it's removed. pkg.go.dev won't show it, and some IDEs and linters will flag use of deprecated functions.
nsrip-dd
changed the title
|
Not a PITA at all :) I think it's reasonable to deprecate it first. I've changed the PR to do so. |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Deprecates the PprofDiff function. It will be removed in v1.50.0.
Motivation
The PprofDiff function was added to our public API unintentionally as
part of #1511. The function is only meant for internal use. This was
missed during code review, likely due to the size of the change and the
fact that most of our attention was devoted to making sure the core
parts of the change were implemented correctly.
This is an exception to our normal v1 compatibility guarantees. It is
very unlikely that any of our users depend on this function. It serves
no purpose in the context of the profiler's public API, and anybody who
might have a reason to use this function would probably have already
found a way to do what they want using the
github.com/google/pprof/profile package.
Describe how to test/QA your changes
N/A
Reviewer's Checklist
Triagemilestone is set.