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

Identification tunings in TileDB and Shapefile drivers #9170

Merged
merged 2 commits into from
Feb 18, 2024

Conversation

rouault
Copy link
Member

@rouault rouault commented Jan 31, 2024

@coveralls
Copy link
Collaborator

Coverage Status

coverage: 68.795% (-0.003%) from 68.798%
when pulling dc1e4d1 on rouault:identification_tunings
into fb3667f on OSGeo:master.

STARTS_WITH_CI(poOpenInfo->pszFilename, "/VSIS3/") ||
STARTS_WITH_CI(poOpenInfo->pszFilename, "/VSIGS/");
// If this is a /vsi virtual file systems, bail out, except if it is S3 or GS.
if (!bIsS3OrGS && STARTS_WITH(poOpenInfo->pszFilename, "/vsi"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a reason for not checking case insensitive here too? (assuming that STARTS_WITH and STARTS_WITH_CI are what I think they are regarding case sensitivity).

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the checking for VSIS3 and VSIGS should have been case sensitive from the beginning, but for some reason the original contributor did it case insensitive, so I've kept that check as it in case it was depended upon

const bool bIsS3OrGS = STARTS_WITH_CI(poOpenInfo->pszFilename, "/VSIS3/") ||
STARTS_WITH_CI(poOpenInfo->pszFilename, "/VSIGS/");
// If this is a /vsi virtual file systems, bail out, except if it is S3 or GS.
if (!bIsS3OrGS && STARTS_WITH(poOpenInfo->pszFilename, "/vsi"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

@elpaso
Copy link
Collaborator

elpaso commented Feb 1, 2024

does this changeset deserve a test?

@rouault
Copy link
Member Author

rouault commented Feb 1, 2024

does this changeset deserve a test?

the existing tests should be good enough IMHO

@rouault
Copy link
Member Author

rouault commented Feb 1, 2024

@normanb Is there any strong reason for the TileDB driver to support /VSIS3/foo spelled like that ? Normally GDAL only recognize the name of the virtual file system in lower case : /vsis3/foo

@rouault rouault merged commit 6d884a6 into OSGeo:master Feb 18, 2024
32 checks passed
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.

None yet

3 participants