Skip to content

refactor: update blas/ext/base/drev to follow current project conventions #1839

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

Merged
merged 11 commits into from
Mar 13, 2024

Conversation

performant23
Copy link
Contributor

@performant23 performant23 commented Mar 12, 2024

Resolves #1488.

Description

What is the purpose of this pull request?

This PR refactors blas/ext/base/drev to follow current project conventions, by updating C++ add-on interfaces to C add-on interfaces along with style and simplification changes.

Related Issues

Does this pull request have any related issues?

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

…ions

This commit refactors the namespace to follow current project conventions i.e. C++ add-on interfaces to C add-on interfaces along with style and simplification changes.
@performant23
Copy link
Contributor Author

Hi, I am not able to trace through the error since it's not related to an import but a header file and it seems to be included in the code that utilizes it. If you could guide me on debugging the error, that'd be great (@Planeshifter , @kgryte , @Pranavchiku )!

Thanks!

@Pranavchiku
Copy link
Member

Can you merge latest main to this PR and check?

Signed-off-by: Pranav <85227306+Pranavchiku@users.noreply.github.com>
@Pranavchiku Pranavchiku changed the title refactor: update blas/ext/base/drev to follow current project conventions refactor: update blas/ext/base/drev to follow current project conventions Mar 12, 2024
@Pranavchiku Pranavchiku added the BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). label Mar 12, 2024
@performant23
Copy link
Contributor Author

Can you merge latest main to this PR and check?

Hi @Pranavchiku, you mean the develop branch right?

Copy link
Member

@Pranavchiku Pranavchiku 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 @performant23 !

@performant23
Copy link
Contributor Author

performant23 commented Mar 12, 2024

@Pranavchiku, Thank you so much for your review, I have pulled the latest changes into the PR!

cc @Planeshifter.

@kgryte kgryte added Native Addons Issue involves or relates to Node.js native add-ons. C Issue involves or relates to C. labels Mar 13, 2024
Signed-off-by: Athan <kgryte@gmail.com>
kgryte added 2 commits March 12, 2024 22:39
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.

LGTM. Thanks, @performant23 and @Pranavchiku.

@kgryte
Copy link
Member

kgryte commented Mar 13, 2024

I am not sure the cause of the failing builds when running C examples.

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.

This should be good now. I figured out the examples error. Namely, the manifest.json file was not fully updated to handle the different build configurations.

@kgryte kgryte merged commit c81e201 into stdlib-js:develop Mar 13, 2024
@kgryte kgryte added the Enhancement Issue or pull request for enhancing existing functionality. label Mar 13, 2024
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/drev to follow current project conventions
3 participants