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

Added a more ergonomic means of accessing GDAL version properties. #305

Merged
merged 7 commits into from Sep 21, 2022

Conversation

metasim
Copy link
Contributor

@metasim metasim commented Sep 3, 2022

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

This PR is of minor impact, but does provide some quality-of-life benefits when building systems which need to dynamically check and verify the version of GDAL available at runtime.

GDAL's native query mechanism uses magical tokens that requires either brute memory, or repeated digging through API docs to fetch the particular version property needed. This PR aims to provide a slightly more principled approach to querying this information via a new enum called VersionInfo. It does not change the existing mechanism, only provides an alternative.

@metasim metasim requested review from lnicola and jdroenner and removed request for lnicola September 5, 2022 14:12
@lnicola
Copy link
Member

lnicola commented Sep 7, 2022

I know that you wanted to make debug-printing to work, but it seems quite convoluted for what it does. I had to go over it three times to spot the version_info call in the Display impl. I'd be fine with a more lightweight approach like:

pub struct VersionInfo(pub &'static str);

impl VersionInfo {
    const RELEASE_NAME: VersionInfo = VersionInfo("RELEASE_NAME");
    // ...

    fn get_version_info(&self) -> String { todo!() }

    fn all() -> String { todo!() }
}

(or even just a pair of functions and some constants, without any struct)

Let's see how the others feel about it.

* master:
  PR feedback.
  cargo fmt
  Enabled docs.rs feature gate flag rendering.
  Clarification of GDAL Programs module.
  Initial draft of Driver documentation.
  Lead-in to Raster and Vector sub-modules.
  Fixed rustdoc warnings in mdarray.
  Super slimmed down intro example.
  Simplifying root module docs.
  Added metadata example.
  Introduced Driver API.
@metasim
Copy link
Contributor Author

metasim commented Sep 9, 2022

@lnicola I turned down the "clever" volume and implemented a more direct approach. What do you think?

@lnicola
Copy link
Member

lnicola commented Sep 9, 2022

Sure, looks good. r? @jdroenner

@metasim metasim changed the title Added a more ergonomic and strongly typed means of accessing GDAL version properties. Added a more ergonomic means of accessing GDAL version properties. Sep 9, 2022
@metasim
Copy link
Contributor Author

metasim commented Sep 21, 2022

bors r=lnicola

@bors
Copy link
Contributor

bors bot commented Sep 21, 2022

🔒 Permission denied

Existing reviewers: click here to make metasim a reviewer

@lnicola
Copy link
Member

lnicola commented Sep 21, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 21, 2022

Build succeeded:

@bors bors bot merged commit b495279 into georust:master Sep 21, 2022
@metasim metasim deleted the feature/version-info-helpers branch September 21, 2022 13:24
@metasim metasim mentioned this pull request Oct 6, 2022
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

2 participants