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

stabilize packages features #35742

Closed
wants to merge 1 commit into from

Conversation

MylesBorins
Copy link
Contributor

These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

Depends on #35741

/cc @nodejs/modules

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/modules

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 21, 2020

When a package has an [`"exports"`][] field, this will take precedence over the
`"main"` field when importing the package by name.

Copy link
Member

Choose a reason for hiding this comment

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

I think @guybedford deliberately wanted this below "exports" because we want people to be preferring "exports" over "main".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the ordering is changed in #35741

I can update it there

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not tied to the ordering, it just a suggestion previously since there was no explicit ordering (it not being alphabetical).

Copy link
Member

Choose a reason for hiding this comment

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

Maybe the list on L825 can have its ordering stated ("In order of precedence, the possible entries are:") while the headers can be in alphabetical order?

doc/api/packages.md Show resolved Hide resolved
@codecov-io

This comment has been minimized.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@Trott
Copy link
Member

Trott commented Oct 22, 2020

This probably shouldn't land before the runtime deprecation in #35747 because otherwise that runtime deprecation is semver-major?

@Trott Trott added the blocked PRs that are blocked by other issues or PRs. label Oct 22, 2020
doc/api/packages.md Outdated Show resolved Hide resolved
@MylesBorins
Copy link
Contributor Author

@Trott the folder pathing stuff is currently no longer documented and unaffected by this change

These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564
@MylesBorins MylesBorins removed the blocked PRs that are blocked by other issues or PRs. label Oct 26, 2020
@MylesBorins
Copy link
Contributor Author

@nodejs/modules I would like to land this. If there are no objections within 24 hours I will land it tomorrow.

@Trott I removed your blocked label as I don't believe the feature you pointed out is affected by this change. Please let me know if you think I'm mistaken

@MylesBorins
Copy link
Contributor Author

Landed in acdfc16

@MylesBorins MylesBorins deleted the mark-features-stable branch October 29, 2020 14:24
MylesBorins added a commit that referenced this pull request Oct 29, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this pull request Nov 3, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@targos targos mentioned this pull request Nov 3, 2020
@guybedford guybedford mentioned this pull request Nov 23, 2020
3 tasks
@MylesBorins MylesBorins added baking-for-lts PRs that need to wait before landing in a LTS release. and removed dont-land-on-v14.x labels Nov 24, 2020
BethGriggs pushed a commit that referenced this pull request Dec 8, 2020
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 29, 2021
These features are being used in production and are ready to be
considered stable.

Refs: nodejs/modules#564

PR-URL: #35742
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
@targos targos removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.