util: fix util.getCallSites plurality#55626
Conversation
|
Review requested:
|
RafaelGSS
left a comment
There was a problem hiding this comment.
What about using a safe approach and keep a getCallSite function emitting a warning to inform users to use getCallSites instead? We can land it as semver-minor and then in the next release we remove the getCallSite completely
`util.getCallSite` returns an array of call site objects. Rename the function to reflect that it returns a given count of frames captured as an array of call site object. Renames the first parameter `frames` to be `frameCount` to indicate that it specifies the count of returned call sites.
bae915c to
1e26b14
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #55626 +/- ##
==========================================
- Coverage 88.43% 88.43% -0.01%
==========================================
Files 654 654
Lines 187718 187725 +7
Branches 36129 36140 +11
==========================================
+ Hits 166010 166012 +2
+ Misses 14948 14942 -6
- Partials 6760 6771 +11
|
| ### DEP0186: `util.getCallSite` | ||
|
|
||
| <!-- YAML | ||
| changes: | ||
| - version: REPLACEME | ||
| pr-url: https://github.com/nodejs/node/pull/55626 | ||
| description: Runtime deprecation. | ||
| --> | ||
|
|
||
| Type: Runtime | ||
|
|
||
| The `util.getCallSite` API has been removed. Please use [`util.getCallSites()`][] instead. |
There was a problem hiding this comment.
Doesn't this have to be doc deprecated first?
There was a problem hiding this comment.
This is an API in active development status, not stable yet. I don't think it is necessary to document deprecated.
There was a problem hiding this comment.
We should not deprecate experimental APIs, deprecation is for stable features we want to phase out.
There was a problem hiding this comment.
There were deprecation for experimental APIs like https://github.com/nodejs/node/blob/main/doc/api/deprecations.md#dep0173-the-assertcalltracker-class.
I was hesistant about adding a deprecation code for this. But all our toolings like expectWarning expects a deprecation code.
|
The
notable-change
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section. |
Co-authored-by: Aviv Keller <redyetidev@gmail.com>
|
Landed in 68dc15e |
util.getCallSitereturns an array of call site objects. Rename thefunction to reflect that it returns a given count of frames captured
as an array of call site object.
Renames the first parameter
framesto beframeCountto indicatethat it specifies the count of returned call sites.
Given that this function is marked as "active development", I believe it is better to rename it early.