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

util: fix util.getCallSites plurality #55626

Merged
merged 3 commits into from
Nov 2, 2024

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Oct 31, 2024

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.

Given that this function is marked as "active development", I believe it is better to rename it early.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Oct 31, 2024
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

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.
Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.43%. Comparing base (4379dfb) to head (b36edf2).
Report is 61 commits behind head on main.

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     
Files with missing lines Coverage Δ
lib/util.js 97.32% <100.00%> (+0.03%) ⬆️
src/node_util.cc 85.76% <100.00%> (ø)

... and 28 files with indirect coverage changes

@RafaelGSS RafaelGSS added the deprecations Issues and PRs related to deprecations. label Oct 31, 2024
Comment on lines +3764 to +3775
### 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.
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this have to be doc deprecated first?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is an API in active development status, not stable yet. I don't think it is necessary to document deprecated.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should not deprecate experimental APIs, deprecation is for stable features we want to phase out.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

@RedYetiDev RedYetiDev added semver-minor PRs that contain new features and should be released in the next minor version. notable-change PRs with changes that should be highlighted in changelogs. labels Oct 31, 2024
Copy link
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @RedYetiDev.

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.

lib/util.js Outdated Show resolved Hide resolved
@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 1, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 1, 2024
@nodejs-github-bot
Copy link
Collaborator

@marco-ippolito marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 2, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 2, 2024
@nodejs-github-bot nodejs-github-bot merged commit 68dc15e into nodejs:main Nov 2, 2024
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 68dc15e

louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
`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.

PR-URL: nodejs#55626
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
aduh95 pushed a commit that referenced this pull request Nov 16, 2024
`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.

PR-URL: #55626
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 19, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626

PR-URL: TODO
RafaelGSS pushed a commit that referenced this pull request Nov 19, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626

PR-URL: #55921
RafaelGSS added a commit that referenced this pull request Nov 19, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626

PR-URL: #55921
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: nodejs#1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: nodejs#1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: nodejs#1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: nodejs#1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: nodejs#1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: https://github.com/nodejs/aduh95/pull/1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: #1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: #1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: #1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: #1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: #1
aduh95 pushed a commit to aduh95/node that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: #1
RafaelGSS added a commit that referenced this pull request Nov 20, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626

PR-URL: #55921
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
`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.

PR-URL: nodejs#55626
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: nodejs#55921
github-actions bot pushed a commit to RafaelGSS/node that referenced this pull request Nov 22, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
sqlite:
  * (SEMVER-MINOR) add support for SQLite Session Extension (Bart Louwers) nodejs#54181
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: TODO
github-actions bot pushed a commit to RafaelGSS/node that referenced this pull request Nov 22, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
sqlite:
  * (SEMVER-MINOR) add support for SQLite Session Extension (Bart Louwers) nodejs#54181
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: TODO
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Nov 26, 2024
Notable changes:

doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732
src:
  * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697
util:
  * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626

PR-URL: nodejs#55921
ruyadorno pushed a commit that referenced this pull request Nov 27, 2024
`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.

PR-URL: #55626
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
ruyadorno pushed a commit that referenced this pull request Nov 27, 2024
`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.

PR-URL: #55626
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
ruyadorno pushed a commit that referenced this pull request Nov 27, 2024
`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.

PR-URL: #55626
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
github-actions bot pushed a commit that referenced this pull request Nov 27, 2024
Notable changes:

assert:
  * (SEMVER-MINOR) make assertion_error use Myers diff algorithm (Giovanni Bucci) #54862
buffer:
  * (SEMVER-MINOR) make Buffer work with resizable ArrayBuffer (James M Snell) #55377
crypto:
  * update root certificates to NSS 3.104 (Richard Lau) #55681
doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732
  * add jazelly to collaborators (Jason Zhang) #55531
esm:
  * mark import attributes and JSON module as stable (Nicolò Ribaudo) #55333
http:
  * (SEMVER-MINOR) add diagnostic channel `http.client.request.created` (Marco Ippolito) #55586
lib:
  * (SEMVER-MINOR) add UV_UDP_REUSEPORT for udp (theanarkh) #55403
module:
  * (SEMVER-MINOR) unflag --experimental-require-module (Joyee Cheung) #55085
net:
  * (SEMVER-MINOR) add UV_TCP_REUSEPORT for tcp (theanarkh) #55408
sqlite:
  * (SEMVER-MINOR) add support for SQLite Session Extension (Bart Louwers) #54181
tools:
  * fix root certificate updater (Richard Lau) #55681
util:
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626

PR-URL: TODO
github-actions bot pushed a commit that referenced this pull request Nov 27, 2024
Notable changes:

assert:
  * (SEMVER-MINOR) make assertion_error use Myers diff algorithm (Giovanni Bucci) #54862
buffer:
  * (SEMVER-MINOR) make Buffer work with resizable ArrayBuffer (James M Snell) #55377
crypto:
  * update root certificates to NSS 3.104 (Richard Lau) #55681
doc:
  * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732
  * add jazelly to collaborators (Jason Zhang) #55531
esm:
  * mark import attributes and JSON module as stable (Nicolò Ribaudo) #55333
http:
  * (SEMVER-MINOR) add diagnostic channel `http.client.request.created` (Marco Ippolito) #55586
lib:
  * (SEMVER-MINOR) add UV_UDP_REUSEPORT for udp (theanarkh) #55403
module:
  * (SEMVER-MINOR) unflag --experimental-require-module (Joyee Cheung) #55085
net:
  * (SEMVER-MINOR) add UV_TCP_REUSEPORT for tcp (theanarkh) #55408
sqlite:
  * (SEMVER-MINOR) add support for SQLite Session Extension (Bart Louwers) #54181
tools:
  * fix root certificate updater (Richard Lau) #55681
util:
  * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626

PR-URL: #56040
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. experimental Issues and PRs related to experimental features. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants