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

Create Subarray object library. #5327

Merged
merged 2 commits into from
Oct 4, 2024
Merged

Create Subarray object library. #5327

merged 2 commits into from
Oct 4, 2024

Conversation

bekadavis9
Copy link
Contributor

@bekadavis9 bekadavis9 commented Sep 30, 2024

Create Subarray object library.

[sc-53884]


TYPE: NO_HISTORY
DESC: Create Subarray object library.

Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

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

I noticed that moving the subarray tests to standalone was reverted because:

  • The find heap API violations workflow failed because of some usages of unique_ptr in the tests. You can either migrate them to use tdb_unique_ptr or update the script to exclude test code (I wouldn't have a problem with it personally).
  • There were some warnings-turned-errors about usages of the deprecated dump C APIs (which happened inside the deprecated C++ APIs and it doesn't make sense to warn if you'd ask me). You can fix them by adding a #define TILEDB_REMOVE_DEPRECATIONS at the top of the files.

It would be nice if you could move the tests in this PR.

@bekadavis9
Copy link
Contributor Author

@teo-tsirpanis I decided it would be best not to come up with a potentially-hacky fix for the heap violations issue. I wanted to address that in a separate PR, with a more well thought-out design. Further, none of the other OL PRs have yet migrated unit tests, so the behavior was inconsistent anyway. It's best to defer the work until the heap issue can be properly resolved, and then the unit test migration can be its own unit of work.

@teo-tsirpanis
Copy link
Member

Sounds good.

Copy link
Member

@teo-tsirpanis teo-tsirpanis left a comment

Choose a reason for hiding this comment

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

Thanks!

@bekadavis9 bekadavis9 merged commit fed218a into dev Oct 4, 2024
63 checks passed
@bekadavis9 bekadavis9 deleted the rd/OLs-subarray branch October 4, 2024 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants