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

Add PolarClass implementation for uuid::Uuid #554

Merged
merged 17 commits into from
Dec 1, 2020
Merged

Conversation

johnhalbert
Copy link
Contributor

@johnhalbert johnhalbert commented Nov 26, 2020

Adds implementation for foreign type uuid::Uuid. This is behind a feature gate uuid. Tests have been added to the /languages/rust/oso/tests/test_polar_rust.rs suite. Tests require running cargo test with --features uuid option. (e.g. $ cargo test --features uuid)

Note: Cargo.toml in /languages/rust/oso/ specifies version 0.6.5 of uuid. The latest version is 0.8.1 at the time of writing, but the version currently used in diesel orm is 0.6.5. Because of the popularity of diesel, and the likelihood of persisting UUID's to the database in a web application context, I used 0.6.5 for compatibility with diesel.

PR checklist:

  • Added changelog entry.

@github-actions
Copy link

github-actions bot commented Nov 26, 2020

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@johnhalbert
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Member

@samscott89 samscott89 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! This looks mostly good, just a few small changes and then this is good to go.

languages/rust/oso/src/lib.rs Outdated Show resolved Hide resolved
languages/rust/oso/src/lib.rs Outdated Show resolved Hide resolved
johnhalbert and others added 2 commits November 29, 2020 16:43
- Rely on default implementation for get_polar_class on Uuid PolarClass
trait impl
- Format get_polar_class_builder with rustfmt
languages/rust/oso/Cargo.toml Outdated Show resolved Hide resolved
languages/rust/oso/Cargo.toml Outdated Show resolved Hide resolved
languages/rust/oso/src/lib.rs Outdated Show resolved Hide resolved
johnhalbert and others added 10 commits November 30, 2020 11:32
- Add diesel mod to provide impl of PolarClass for Uuid
- Rename uuid crate to uuid_v06
- Rename uuid feature to uuid_v06
- Rename extra feature group to diesel
- Update PolarClass impl for Uuid to use full path for foreign types
We can consider adding a "diesel" group if diesel requires additional
PolarClass impls, but for now I think we're good with a single feature
flag for a single impl.
@gj gj dismissed samscott89’s stale review December 1, 2020 06:10

Comments addressed

@gj gj merged commit 2d5b302 into osohq:main Dec 1, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 1, 2020
@gj
Copy link
Member

gj commented Dec 1, 2020

Thanks again @johnhalbert ! Credited you in the changelog 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants