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

zlib: remove prototype primordials usage #54695

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Sep 1, 2024

As a result of the primordials work done by @aduh95 and others, let's remove the prototypal primordials from node:zlib

@anonrig anonrig requested a review from aduh95 September 1, 2024 16:28
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Sep 1, 2024
@avivkeller
Copy link
Member

CC @nodejs/primordials @nodejs/zlib

@avivkeller avivkeller added the needs-benchmark-ci PR that need a benchmark CI run. label Sep 1, 2024
@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 1, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 1, 2024
@nodejs-github-bot

This comment was marked as outdated.

lib/zlib.js Outdated Show resolved Hide resolved
@anonrig anonrig added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Sep 1, 2024
Copy link

codecov bot commented Sep 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.07%. Comparing base (c77bcf0) to head (3bf4e0a).
Report is 520 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54695      +/-   ##
==========================================
- Coverage   88.07%   88.07%   -0.01%     
==========================================
  Files         652      652              
  Lines      183570   183566       -4     
  Branches    35872    35860      -12     
==========================================
- Hits       161686   161681       -5     
- Misses      15137    15140       +3     
+ Partials     6747     6745       -2     
Files with missing lines Coverage Δ
lib/zlib.js 98.41% <100.00%> (-0.01%) ⬇️

... and 41 files with indirect coverage changes

@nodejs-github-bot

This comment was marked as outdated.

lib/zlib.js Show resolved Hide resolved
lib/zlib.js Outdated Show resolved Hide resolved
doc/contributing/primordials.md Show resolved Hide resolved
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@jasnell
Copy link
Member

jasnell commented Sep 8, 2024

This PR is currently blocked from landing due to unreliable CI

@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 9, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 9, 2024
@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig force-pushed the yagiz/remove-primordials-zlib branch from 61cd7b9 to a77b6cb Compare September 10, 2024 15:01
@anonrig anonrig requested a review from jasnell September 10, 2024 23:52
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

anonrig and others added 2 commits September 15, 2024 16:06
# Conflicts:
#	lib/zlib.js
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@anonrig anonrig force-pushed the yagiz/remove-primordials-zlib branch from a77b6cb to 3bf4e0a Compare September 15, 2024 20:06
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 19, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 19, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/54695
✔  Done loading data for nodejs/node/pull/54695
----------------------------------- PR info ------------------------------------
Title      zlib: remove prototype primordials usage (#54695)
Author     Yagiz Nizipli <yagiz@nizipli.com> (@anonrig)
Branch     anonrig:yagiz/remove-primordials-zlib -> nodejs:main
Labels     zlib, author ready, needs-ci, needs-benchmark-ci, commit-queue-squash
Commits    2
 - zlib: remove prototype primordials usage
 - Update zlib.js
Committers 1
 - Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/54695
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/54695
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - zlib: remove prototype primordials usage
   ⚠  - Update zlib.js
   ℹ  This PR was created on Sun, 01 Sep 2024 16:28:21 GMT
   ✔  Approvals: 7
   ✔  - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/54695#pullrequestreview-2274458346
   ✔  - Marco Ippolito (@marco-ippolito) (TSC): https://github.com/nodejs/node/pull/54695#pullrequestreview-2274459245
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/54695#pullrequestreview-2277737863
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/54695#pullrequestreview-2277787527
   ✔  - Ruy Adorno (@ruyadorno) (TSC): https://github.com/nodejs/node/pull/54695#pullrequestreview-2283877520
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/54695#pullrequestreview-2289797393
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/54695#pullrequestreview-2295411895
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-09-18T23:57:25Z: https://ci.nodejs.org/job/node-test-pull-request/62543/
- Querying data for job/node-test-pull-request/62543/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/10933395608

@anonrig anonrig requested a review from KhafraDev September 19, 2024 02:38
@anonrig anonrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Sep 19, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 19, 2024
@nodejs-github-bot nodejs-github-bot merged commit c42d846 into nodejs:main Sep 19, 2024
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in c42d846

targos pushed a commit that referenced this pull request Oct 4, 2024
# Conflicts:
#	lib/zlib.js

PR-URL: #54695
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
@aduh95 aduh95 mentioned this pull request Oct 9, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
# Conflicts:
#	lib/zlib.js

PR-URL: nodejs#54695
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
@marco-ippolito marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Nov 16, 2024
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
# Conflicts:
#	lib/zlib.js

PR-URL: nodejs#54695
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. needs-benchmark-ci PR that need a benchmark CI run. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.