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

Doc: OpenFileGDB: document OPENFILEGDB_DEFAULT_STRING_WIDTH configuration option (fixes #9165) #9166

Merged
merged 2 commits into from
Feb 1, 2024

Conversation

rouault
Copy link
Member

@rouault rouault commented Jan 30, 2024

No description provided.

@coveralls
Copy link
Collaborator

coveralls commented Jan 30, 2024

Coverage Status

coverage: 68.796% (-0.009%) from 68.805%
when pulling 6a9caf8 on rouault:fix_9165
into 5d6bbd0 on OSGeo:master.

@@ -664,7 +664,7 @@ int OGROpenFileGDBLayer::BuildLayerDefinition()
// string width is 0, we pick up 65536 by default to mean unlimited
Copy link
Collaborator

Choose a reason for hiding this comment

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

this comment needs updates, maybe

Suggested change
// string width is 0, we pick up 65536 by default to mean unlimited
// string width is 0, if OPENFILEGDB_DEFAULT_STRING_WIDTH is not set we pick up 65536 by default to mean unlimited

@elpaso
Copy link
Collaborator

elpaso commented Feb 1, 2024

I find it confusing that the C++ constant OPENFILEGDB_DEFAULT_STRING_WIDTH and the open option have the same name: if I read this right the first is the hard limit (and the default) and the second is a user-defined default which still needs to be less than the hard limit.

@rouault
Copy link
Member Author

rouault commented Feb 1, 2024

I find it confusing that the C++ constant OPENFILEGDB_DEFAULT_STRING_WIDTH

yes, I agree that's confusing. Renamed in an extra commit where I've also added extensive comments to explain the trick

@rouault
Copy link
Member Author

rouault commented Feb 1, 2024

(note: CIFuzz failure is unrelated, and will be addressed per #9181)

@rouault rouault merged commit 85c93fd into OSGeo:master Feb 1, 2024
31 of 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.

3 participants