-
-
Notifications
You must be signed in to change notification settings - Fork 594
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
feat: update namespace exports #1386
Conversation
* @type {Function} | ||
* @see {@link module:@stdlib/ndarray/dispatch-by} | ||
*/ | ||
setReadOnly( ns, 'dispatchBy', require( '@stdlib/ndarray/dispatch-by' ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be included.
* @type {Function} | ||
* @see {@link module:@stdlib/random/exponential} | ||
*/ | ||
setReadOnly( ns, 'exponential', require( '@stdlib/random/exponential' ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be exported until finalized.
* @type {Function} | ||
* @see {@link module:@stdlib/strided/dispatch-by} | ||
*/ | ||
setReadOnly( strided, 'dispatchBy', require( '@stdlib/strided/dispatch-by' ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be exported until finalized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Planeshifter Did you manually run this workflow? I think we should just manually do this locally, as this is picking up things which should not be exported to namespaces as they are effectively WIPs.
@kgryte No; I changed the workflow and ESLint rule to actually apply the suggestions, which never worked previously. I think it's easier to just automate this, but am aware that we have a bunch of packages we don't want to export for now. However, we already support the following comment syntax
Why don't we just list the respective packages there for the time being? Can add a |
@Planeshifter Okay, but we also need to remember to add some of these packages to the REPL namespace. I typically use adding to the tree namespace as a way to remind myself to also add to the REPL namespace, so, for me, the lack of automation is intentional. |
FWIW, I am okay with the namespace export rule just emitting warnings, rather than attempting to autofix. |
Just for context, the underlying lint rule won't attempt to auto-fix by default or when committing changes to any of the files, only in this specific workflow due to the fix type being set to cover suggestions:
As for the REPL namespace, what about having a workflow that updates a tracking issue with all packages currently not exported to |
The tracking issue would likely be verrrry noisy. We should definitely have tooling for checking what is and what is not exported to the REPL, but I don't think a workflow and tracking issue is particularly useful. |
eadb9de
to
e052dbf
Compare
e90a991
to
fab4044
Compare
e6fbaac
to
800cdf0
Compare
Signed-off-by: stdlib-bot <[email protected]>
800cdf0
to
fc3fdbb
Compare
This PR