Fix review comments

.github/workflows/rust.yml

@@ -2,13 +2,15 @@ name: Rust
 
 on:
   push:
-    branches: [ "main" ]
+    branches: ["main"]
+  pull_request:
+    branches: ["*"]
 
 env:
   CARGO_TERM_COLOR: always
 
 jobs:
-  build:
+  test:
     runs-on: ubuntu-latest
 
     steps:
@@ -21,7 +23,7 @@ jobs:
       - name: Test
         run: cargo test
 
-  build-legacy:
+  test-legacy:
     runs-on: ubuntu-latest
 
     steps:

Cargo.toml

@@ -1,7 +1,7 @@
 [workspace]
 members = ["test_utils"]
 exclude = ["fuzz"]
-# 2 so msrv does not hold up dependence versions like in not 3
+# v2 so msrv does not hold up dependence versions like in v3
 resolver = "2"
 
 [package]
@@ -13,7 +13,7 @@ rust-version = "1.85.1"
 
 repository = "https://github.com/glennDittmann/geogram_predicates"
 authors = ["Glenn Dittmann", "TimTheBig"]
-license = "LGPL-3.0"
+license = "LGPL-3.0 OR MIT"
 
 keywords = ["computer-graphics", "math", "geometry", "predicates", "robust"]
 categories = ["no-std::no-alloc", "mathematics", "graphics"]
@@ -29,9 +29,8 @@ cxx = { version = "1.0", features = ["alloc", "c++20"], default-features = false
 cxx-build = { version = "1.0", optional = true }
 
 [dev-dependencies]
-float_extras = "0.1.6"
+float_extras = "0.1"
 test_utils = { path = "test_utils" }
 
 [features]
-alloc = ["cxx?/alloc"]
-legacy = ["dep:cxx", "dep:cxx-build", "alloc"]
+legacy = ["dep:cxx", "dep:cxx-build"]

README.md

@@ -11,7 +11,7 @@ Be sure to check it out [here](https://github.com/BrunoLevy/geogram).
 It yields easy access to dependency-free parts of its code base, as so called _Pluggable Software Modules_ (PSM), which in turn make it easy to write `cxx_bridges` for these.
 </details>
 
-This library is licenced under the LGPL-3.0, when the legacy feature is enabled the c++ code it uses is under Apache-2.0.
+This library is licenced under the `LGPL-3.0 OR MIT`, when the legacy feature is enabled the c++ code it uses is under `Apache-2.0`.
 
 ## Example
 
@@ -71,7 +71,7 @@ Below are visualizations comparing naive and robust `orient_2d` & `in_circle_2d`
 
 ### Other
 - [x] det_4d() (not in rust)
-- [x] geo_sgn()
+- [x] geo_sgn() (renamed to geo_sign)
 - [x] initialize() (not in rust)
 - [x] show_stats() (not in rust)
 - [x] terminate() (not in rust)

src/expansion/expansion.rs

@@ -24,7 +24,6 @@ impl<const N: usize> fmt::Display for Expansion<N> {
 }
 
 impl<const N: usize> PartialEq for Expansion<N> {
-    // todo check if this should be `self.data == other.data`
     fn eq(&self, other: &Self) -> bool {
         self.equals(other)
     }

src/types.rs

@@ -1,6 +1,9 @@
 use core::cmp::Ordering;
 
-/// A helper for functions that return signs
+/// A helper for functions that return signs.
+/// 
+/// Note: Sign's memory layout is `repr(i8)` so it can be freely converted to i8,
+/// and back in some cases like multiply.
 #[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone)]
 #[repr(i8)]
 pub enum Sign {
@@ -54,6 +57,8 @@ impl PartialOrd<i8> for Sign {
 impl core::ops::Mul for Sign {
     type Output = Sign;
 
+    // It's more efficient then the safe alternative a match,
+    // as a integer multiply is faster then a 9 branch match.
     #[inline]
     fn mul(self, rhs: Self) -> Self::Output {
         // SAFETY: Sign is repr(i8) and will be (-1, 0, 1) so the product must be (1, 0, -1)