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

refactor: update blas/ext/base/sapx to follow current project conventions #1824

Merged
merged 14 commits into from
Mar 21, 2024

Conversation

rxbryan
Copy link
Contributor

@rxbryan rxbryan commented Mar 11, 2024

update blas/ext/base/sapx to follow current project conventions

Description

updates blas/ext/base/sapx to follow current project conventions as outlined in issue #788

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.


@stdlib-js/reviewers

Copy link
Contributor

@stdlib-bot stdlib-bot left a comment

Choose a reason for hiding this comment

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

👋 Hi there! 👋

And thank you for opening your first pull request! We will review it shortly. 🏃 💨

@Planeshifter Planeshifter changed the title refactor: update blas/ext/base/sapx refactor: update blas/ext/base/sapx Mar 11, 2024
@kgryte kgryte added Needs Review A pull request which needs code review. Native Addons Issue involves or relates to Node.js native add-ons. BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). C Issue involves or relates to C. labels Mar 11, 2024
Copy link
Member

@kgryte kgryte left a 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 @rxbryan. This PR is missing updated examples and is missing updated tests.

@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Mar 11, 2024
@rxbryan
Copy link
Contributor Author

rxbryan commented Mar 12, 2024

This PR is missing updated examples and is missing updated tests.

Thanks for the review @kgryte I will implement the changes ASAP

@rxbryan
Copy link
Contributor Author

rxbryan commented Mar 12, 2024

Hi @kgryte I have pushed the requested changes.

update blas/ext/base/sapx to follow current project conventions

Fixes: stdlib-js#1152
@rxbryan
Copy link
Contributor Author

rxbryan commented Mar 15, 2024

Hi @kgryte Sorry for the excessive styling mistakes but they are all fixed now. I have updated the examples and tests

Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

The manifest.json file needs to be updated. And just checking: there were no changes necessary for docs/repl.txt or docs/types/index.d.ts?

@kgryte kgryte added the Enhancement Issue or pull request for enhancing existing functionality. label Mar 21, 2024
update manifest.json, docs/repl.txt
@rxbryan
Copy link
Contributor Author

rxbryan commented Mar 21, 2024

The manifest.json file needs to be updated. And just checking: there were no changes necessary for docs/repl.txt or docs/types/index.d.ts?

I've updated manifest.json and docs/repl.txt. It did require some changes.

docs/types/index.d.ts looks okay

Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Signed-off-by: Athan <kgryte@gmail.com>
Copy link
Member

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @rxbryan!

@kgryte kgryte removed the Needs Changes Pull request which needs changes before being merged. label Mar 21, 2024
@kgryte kgryte changed the title refactor: update blas/ext/base/sapx refactor: update blas/ext/base/sapx to follow current project conventions Mar 21, 2024
@kgryte kgryte merged commit e4f0dcd into stdlib-js:develop Mar 21, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). C Issue involves or relates to C. Enhancement Issue or pull request for enhancing existing functionality. Native Addons Issue involves or relates to Node.js native add-ons.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC]: refactor blas/ext/base/sapx to follow current project conventions
3 participants