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

buffer: remove TODOs in atob / btoa #38548

Merged
merged 1 commit into from
May 9, 2021
Merged

Conversation

XadillaX
Copy link
Contributor

@XadillaX XadillaX commented May 5, 2021

@XadillaX XadillaX requested a review from jasnell May 5, 2021 07:28
@github-actions github-actions bot added buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run. labels May 5, 2021
@XadillaX XadillaX requested a review from addaleax May 5, 2021 07:28
@Trott
Copy link
Member

Trott commented May 6, 2021

Looks good to me, but I wonder if it might be better to replace the comment with something like:

// This implementation is not optimized and should not be.
// Refs: https://github.com/nodejs/node/pull/38433#issuecomment-828426932

@XadillaX XadillaX requested review from addaleax and Trott May 6, 2021 09:01
@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 7, 2021
@nodejs-github-bot

This comment has been minimized.

@jasnell jasnell removed the needs-ci PRs that need a full CI run. label May 7, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Refs: nodejs#38433 (comment)

PR-URL: nodejs#38548
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@aduh95
Copy link
Contributor

aduh95 commented May 9, 2021

Landed in 0b6f0a0

@aduh95 aduh95 merged commit 0b6f0a0 into nodejs:master May 9, 2021
targos pushed a commit that referenced this pull request May 17, 2021
Refs: #38433 (comment)

PR-URL: #38548
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request May 30, 2021
Refs: #38433 (comment)

PR-URL: #38548
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
Refs: #38433 (comment)

PR-URL: #38548
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
Refs: #38433 (comment)

PR-URL: #38548
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Jun 11, 2021
Refs: #38433 (comment)

PR-URL: #38548
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@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. buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants