-
Notifications
You must be signed in to change notification settings - Fork 6
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
Wrap the imf_version module #20
Merged
anderslanglands
merged 10 commits into
vfx-rs:main
from
scott-wilson:18-bind_imf_version
May 16, 2021
Merged
Wrap the imf_version module #20
anderslanglands
merged 10 commits into
vfx-rs:main
from
scott-wilson:18-bind_imf_version
May 16, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
anderslanglands
approved these changes
May 16, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wunderbar lgtm
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Closes #18.
Implement the
imfVersion
module.This changes the API from what's in C++ to make the version and flags have their own type, instead of being a simple integer.
CPPMM Checklist
Cppmm is our bindings generator. There are some tools in it to aid in creating safe and stable APIs.
CPPMM_RENAME(ctor)
for the "main" type constructor.CPPMM_THROWS
for any code that may throw an exception in C++.FFI Safety Checklist
The Rustonomicon provides useful guides to prevent soundness errors in Rust. The FFI section of the document provides the source for these checklist items.
openexr::version::Version::new(...)
), but this shouldn't be called on the C++ side.CStr
orCString
Box
Rust API Guidelines Checklist
The checklist is heavily inspired by The Rust API Guidelines. Please check each item that applies, or note if an item is intentionally skipped (either partially or fully) with the reason.
as_
,to_
,into_
conventions (C-CONV)iter
,iter_mut
,into_iter
(C-ITER)Copy
,Clone
,Eq
,PartialEq
,Ord
,PartialOrd
,Hash
,Debug
,Display
,Default
From
,AsRef
,AsMut
(C-CONV-TRAITS)FromIterator
andExtend
(C-COLLECT)Serialize
,Deserialize
(C-SERDE)Send
andSync
where possible (C-SEND-SYNC)Hex
,Octal
,Binary
formatting (C-NUM-FMT)R: Read
andW: Write
by value (C-RW-VALUE)libopenexr-c-3_0.so
. It seems to be a rust bug. (Ticket: cargo test --doc not load same shared libraries as cargo test --lib rust-lang/cargo#8531)?
, nottry!
, notunwrap
(C-QUESTION-MARK)readme, keywords, categories
Deref
andDerefMut
(C-DEREF)bool
orOption
(C-CUSTOM-TYPE)bitflags
, not enums (C-BITFLAG)Debug
(C-DEBUG)Debug
representation is never empty (C-DEBUG-NONEMPTY)Testing Checklist
The tests are meant to validate that the wrappers are sound and work as expected. Tests that are designed to exercise code for the wrapped library and not the bindings should be added to the wrapped library instead.
unsafe {}
code is properly sanitized