Skip to content

Conversation

@antaljanosbenjamin
Copy link
Contributor

No description provided.

@antaljanosbenjamin antaljanosbenjamin force-pushed the T059-MAGE-temporal-type-to-rsmgp-sys branch 3 times, most recently from 9adf16b to fc79668 Compare October 14, 2021 06:32
@antaljanosbenjamin antaljanosbenjamin added lang: rust Issue on Rust codebase type: enhancement type: utility Something that everyone should use labels Oct 14, 2021
@antaljanosbenjamin antaljanosbenjamin force-pushed the T059-MAGE-temporal-type-to-rsmgp-sys branch from fc79668 to c90b42d Compare October 14, 2021 07:53
@antaljanosbenjamin antaljanosbenjamin force-pushed the T059-MAGE-temporal-type-to-rsmgp-sys branch from c90b42d to 6dbed22 Compare October 14, 2021 08:09
@antaljanosbenjamin antaljanosbenjamin self-assigned this Oct 14, 2021
@antaljanosbenjamin antaljanosbenjamin marked this pull request as ready for review October 14, 2021 10:59
Copy link
Member

@gitbuda gitbuda left a comment

Choose a reason for hiding this comment

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

🚀 🚀

Comment on lines +290 to +298
pub fn month(&self) -> u32 {
unsafe {
invoke_mgp_func!(i32, ffi::mgp_local_date_time_get_month, self.ptr).unwrap() as u32
}
}

pub fn day(&self) -> u32 {
unsafe { invoke_mgp_func!(i32, ffi::mgp_local_date_time_get_day, self.ptr).unwrap() as u32 }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for casting all of these types to uint, since mgp calls return int?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! I meant these classes to used only to convert from/to the chrono types, therefore I adjusted the return types of these function to match the types of the chrono types, e.g. the usage in to_naive_date_time.

Error::UnableToCreateDurationFromChronoDuration
);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Very extensive tests! 🥇

@jmatak jmatak merged commit 6b58b10 into main Oct 21, 2021
@jmatak jmatak deleted the T059-MAGE-temporal-type-to-rsmgp-sys branch October 21, 2021 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang: rust Issue on Rust codebase type: utility Something that everyone should use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants