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

fs: export constants from fs/promises #43177

Merged
merged 1 commit into from
Jun 3, 2022

Conversation

F3n67u
Copy link
Member

@F3n67u F3n67u commented May 22, 2022

This pr re-export constants from fs/promises

Motivation

see #43140 (comment)

constants is only export from fs, but not export from fs/promises.

When writing code that uses the promise based fs library via fs/promises, if fs constants is used, we have to add another import module which is not convenient:

import fs from 'fs/promise';
import { constants } from 'fs';

If we re-export constants from fs/promises, then we could reduce two import to one:

import fs, { constants } from 'fs/promises';

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels May 22, 2022
@F3n67u F3n67u marked this pull request as ready for review May 22, 2022 15:12
@F3n67u F3n67u force-pushed the fs-constants branch 3 times, most recently from 7a98b0b to 1e353ef Compare May 22, 2022 15:36
lib/fs/promises.js Outdated Show resolved Hide resolved
@F3n67u F3n67u requested a review from LiviaMedeiros May 27, 2022 05:25
@F3n67u F3n67u changed the title fs: re-export constants from fs/promises fs: export constants from fs/promises May 27, 2022
@F3n67u
Copy link
Member Author

F3n67u commented May 27, 2022

@LiviaMedeiros I export constants from lib/internal/fs/promises.js instead of adding property at lib/fs/promises.js, please take another look.

@LiviaMedeiros
Copy link
Contributor

Code looks good to me. Shouldn't documentation be updated accordingly?

@F3n67u F3n67u force-pushed the fs-constants branch 2 times, most recently from 7aa855f to 2e1197e Compare May 27, 2022 14:06
@F3n67u
Copy link
Member Author

F3n67u commented May 27, 2022

Code looks good to me. Shouldn't documentation be updated accordingly?

Done. this is the first time to update the Node.js document. please correct me if it is not updated correctly.

doc/api/fs.md Outdated Show resolved Hide resolved
@LiviaMedeiros LiviaMedeiros 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 May 29, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 29, 2022
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 3, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 3, 2022
@nodejs-github-bot nodejs-github-bot merged commit f765693 into nodejs:master Jun 3, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in f765693

@F3n67u F3n67u deleted the fs-constants branch June 3, 2022 08:56
italojs pushed a commit to italojs/node that referenced this pull request Jun 6, 2022
PR-URL: nodejs#43177
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 11, 2022
PR-URL: #43177
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@danielleadams danielleadams mentioned this pull request Jun 11, 2022
danielleadams pushed a commit that referenced this pull request Jun 13, 2022
PR-URL: #43177
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
PR-URL: #43177
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
PR-URL: #43177
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
PR-URL: nodejs/node#43177
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Nov 21, 2022
PR-URL: #45556
Refs: #43177
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Nov 23, 2022
PR-URL: nodejs#45556
Refs: nodejs#43177
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
ruyadorno pushed a commit that referenced this pull request Nov 24, 2022
PR-URL: #45556
Refs: #43177
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45556
Refs: #43177
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45556
Refs: #43177
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
PR-URL: #45556
Refs: #43177
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
PR-URL: #45556
Refs: #43177
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 5, 2023
PR-URL: #45556
Refs: #43177
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@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. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants