-
-
Notifications
You must be signed in to change notification settings - Fork 835
refactor: update blas/ext/base/dapxsum
to follow current project conventions
#1793
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
Conversation
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
lib/node_modules/@stdlib/blas/ext/base/dapxsum/examples/index.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Athan <[email protected]>
lib/node_modules/@stdlib/blas/ext/base/dapxsum/lib/ndarray.native.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Athan <[email protected]>
@@ -35,6 +35,10 @@ | |||
], | |||
"libpath": [], | |||
"dependencies": [ | |||
"@stdlib/napi/export", |
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 file has mixed TABS and spaces. The indentation should be 2-space.
This file is also missing separate build configurations for benchmarks and examples. See, e.g., https://github.com/stdlib-js/stdlib/blob/develop/lib/node_modules/%40stdlib/blas/ext/base/dapxsumors/manifest.json.
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.
Added the missing confs.
For the indentation thing, I've run the document formatting tool on the file and they all look 2-spaces indented. If still not the correct result, please let me know how to deal with it
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Athan <[email protected]>
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.
Thanks for working on this @Deadreyo. Left a few comments. I also believe that there are missing changes to test files and some other files in lib
. E.g., see this recent PR: https://github.com/stdlib-js/stdlib/pull/1993/files. Once resolved, this PR should be good to move forward.
Updated missing changes in |
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.
Did a review and the changes look all good to me. Thank you for working on this, @Deadreyo.
Signed-off-by: Philipp Burckhardt <[email protected]>
Resolves #1465 .
Description
This pull request refactors
blas/ext/base/dapxsum
to follow current project conventions.Related Issues
Related issues #788 , #1152
This pull request:
blas/ext/base/dapxsum
to follow current project conventions #1465Questions
No.
Other
No.
Checklist
@stdlib-js/reviewers