Skip to content

Conversation

stdlib-bot
Copy link
Contributor

@stdlib-bot stdlib-bot commented Feb 20, 2025

This PR

  • updates namespace TypeScript declarations

Reviewer Checklist

  • Check the scope of the changes (following Conventional Commits):
    • Are these new APIs? Then this is a feat.
    • Are these changes to existing APIs that could break compatibility? Then this is a feat! (i.e., a breaking change).
    • Are these only documentation changes to existing APIs? Then this is docs.
  • Update the PR title to align with the change type (feat, feat!, or docs).
  • Approve the PR once you are confident about the classification and changes made.

@stdlib-bot stdlib-bot added automated-pr Automated pull request (e.g., from a bot). Documentation Improvements, additions, or changes to documentation. labels Feb 20, 2025
@stdlib-bot stdlib-bot requested a review from a team February 20, 2025 02:29
@stdlib-bot
Copy link
Contributor Author

stdlib-bot commented Feb 20, 2025

Coverage Report

Package Statements Branches Functions Lines
math/base/ops $\color{green}222/222$
$\color{green}+100.00\%$
$\color{green}1/1$
$\color{green}+100.00\%$
$\color{green}0/0$
$\color{green}+100.00\%$
$\color{green}222/222$
$\color{green}+100.00\%$
stats/base $\color{green}1932/1932$
$\color{green}+100.00\%$
$\color{green}1/1$
$\color{green}+100.00\%$
$\color{green}0/0$
$\color{green}+100.00\%$
$\color{green}1932/1932$
$\color{green}+100.00\%$

The above coverage report was generated for the changes in this PR.

Copy link
Member

@Planeshifter Planeshifter left a comment

Choose a reason for hiding this comment

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

@kgryte @aayush0325

I am noticing per this PR that the namespace exports in lib/index.js for both math/base/ops and stats/base now export packages from other namespaces. Is this intentional?

This likely will have undesirable effects due to pulling in new dependencies for various namespaces. Our tooling also doesn't envision this. #5332 removes the packages from the README.md table of contents as they are not part of the namespace package anymore, so documentation and actual package contents are going out of sync.

I would argue that we should remove the moved packages from the old namespaces and provide a migration path to end-users with the next release of the packages.

@github-actions github-actions bot added the Needs Changes Pull request which needs changes before being merged. label Feb 20, 2025
@kgryte
Copy link
Member

kgryte commented Feb 20, 2025

@Planeshifter Yes, this is intentional. The plan is to migrate all the exports to a new namespace in one fell swoop. Otherwise, the migration process becomes more complicated.

@kgryte
Copy link
Member

kgryte commented Feb 20, 2025

@Planeshifter That #5332 removes the packages from the README docs seems like a limitation of the tooling that could be addressed. But also, that is not a blocker for me, as it simply makes the exports "hidden" before we actually migrate the exports to a new namespace.

@kgryte kgryte removed the Needs Changes Pull request which needs changes before being merged. label Feb 20, 2025
@Planeshifter
Copy link
Member

@kgryte Thanks for the clarification!

Planeshifter
Planeshifter previously approved these changes Feb 20, 2025
@stdlib-bot stdlib-bot force-pushed the update-namespace-declarations branch from 3a644b8 to 3fd40ed Compare February 21, 2025 02:29
@stdlib-bot stdlib-bot requested a review from a team February 21, 2025 02:29
@kgryte kgryte changed the title feat: update namespace TypeScript declarations docs: update namespace TypeScript declarations Feb 21, 2025
@kgryte kgryte merged commit 54ed22a into develop Feb 21, 2025
17 checks passed
@kgryte kgryte deleted the update-namespace-declarations branch February 21, 2025 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-pr Automated pull request (e.g., from a bot). Documentation Improvements, additions, or changes to documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants