Skip to content

Conversation

@DimitrisStaratzis
Copy link
Contributor

No description provided.

@DimitrisStaratzis DimitrisStaratzis marked this pull request as draft January 25, 2024 18:19
@DimitrisStaratzis DimitrisStaratzis force-pushed the dstara/update_to_tiledb_2.20 branch from e6fe946 to 3025468 Compare February 6, 2024 11:37
@DimitrisStaratzis DimitrisStaratzis changed the title update to TileDB-2.20.0-rc0 update to TileDB-2.20.0 Feb 6, 2024
@DimitrisStaratzis DimitrisStaratzis marked this pull request as ready for review February 19, 2024 16:23
)

target_compile_definitions(tiledbjni PRIVATE -DTILEDB_CORE_OBJECTS_EXPORTS)
target_compile_definitions(tiledbjni PRIVATE -Dtiledb_EXPORTS)
Copy link
Member

Choose a reason for hiding this comment

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

@DimitrisStaratzis do we actually need this?

@teo-tsirpanis why did this name change?

Copy link
Member

@ihnorton ihnorton Feb 20, 2024

Choose a reason for hiding this comment

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

AFAICT the reason for this -D is to enable TILEDB_EXPORT in the https://github.com/TileDB-Inc/TileDB-Java/blob/master/swig/tiledb_java_extensions.h file. That file is effectively borrowing/piggybacking on the core exports.h (we could generate our own for -Java with cmake). So probably this is the ~only usage.

Copy link
Member

Choose a reason for hiding this comment

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

The define was renamed in TileDB-Inc/TileDB#4528. I wouldn't consider it a breaking change; this is an undocumented implementation detail that no external repository should define. This is in fact the only use of this define.

Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

LGTM other than the cmake question.

@DimitrisStaratzis DimitrisStaratzis merged commit 23221e0 into master Feb 21, 2024
@DimitrisStaratzis DimitrisStaratzis deleted the dstara/update_to_tiledb_2.20 branch February 21, 2024 18:14
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.

4 participants