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

Consolidated fixture! implementation and enabled usage anywhere in module structure. #342

Merged
merged 5 commits into from
Dec 5, 2022

Conversation

metasim
Copy link
Contributor

@metasim metasim commented Nov 20, 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.

There were two versions of fixture!, and was working on a PR that needed a third, so this simplifies and consolidates the implementation by making use of relative paths in [env] section of config.toml, passing the fully qualified path to the fixture directory to the compiler via an environment variable.

src/test_utils.rs Outdated Show resolved Hide resolved
@metasim metasim mentioned this pull request Nov 30, 2022
2 tasks
@lnicola
Copy link
Member

lnicola commented Dec 5, 2022

Removing the duplicated code and turning the macro into a function are good ideas, but I'm not sure what the extra environment variable is bringing.

I suppose we could use it spread the test fixtures around, but we only have two crates now. It seems to me that the current arrangement (with paths relative to the manifest) is more predictable and easier to understand.

@metasim
Copy link
Contributor Author

metasim commented Dec 5, 2022

@lnicola

but I'm not sure what the extra environment variable is bringing

🤔 🤦 I got so wrapped up the old implementation that I missed the "forest for the trees". Yeh, extra indirection is kinda silly.

@metasim
Copy link
Contributor Author

metasim commented Dec 5, 2022

bors r=lnicola

@bors bors bot merged commit d1f5a3a into georust:master Dec 5, 2022
@metasim metasim deleted the feature/temp-fixure++ branch December 5, 2022 18:24
bors bot added a commit that referenced this pull request May 17, 2023
397: Support for accessing the `SpatialRef` of GCPs. r=lnicola a=metasim

- [X] I agree to follow the project's [code of conduct](https://github.com/georust/gdal/blob/master/CODE_OF_CONDUCT.md).
- [X] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
---

Also removes trailing use of old `fixture!` macro originally removed in #342

Co-authored-by: Simeon H.K. Fitch <[email protected]>
bors bot added a commit that referenced this pull request May 17, 2023
397: Support for accessing the `SpatialRef` of GCPs. r=lnicola a=metasim

- [X] I agree to follow the project's [code of conduct](https://github.com/georust/gdal/blob/master/CODE_OF_CONDUCT.md).
- [X] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
---

Also removes trailing use of old `fixture!` macro originally removed in #342

Co-authored-by: Simeon H.K. Fitch <[email protected]>
bors bot added a commit that referenced this pull request May 17, 2023
397: Support for accessing the `SpatialRef` of GCPs. r=lnicola a=metasim

- [X] I agree to follow the project's [code of conduct](https://github.com/georust/gdal/blob/master/CODE_OF_CONDUCT.md).
- [X] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
---

Also removes trailing use of old `fixture!` macro originally removed in #342

Co-authored-by: Simeon H.K. Fitch <[email protected]>
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.

2 participants