martinzink commented on code in PR #2258:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2258#discussion_r3967287946


##########
minifi_rust/minifi_rs_behave/linux_build.sh:
##########
@@ -81,10 +81,19 @@ TARGET_DIR="target/release"
 mkdir -p "$TARGET_DIR"
 
 # 3. Build using Docker
+# When running under GitHub Actions, persist the chef layer across runs

Review Comment:
   Couldnt allow the actions yet because it needs to be enabled in apache org, 
it still needs this actions
   
   ```
         - name: Set up Docker Buildx
           uses: docker/setup-buildx-action@v3
         - name: Expose GitHub Actions cache to buildx
           uses: crazy-max/ghaction-github-runtime@v3
   ```



##########
minifi_rust/extensions/minifi_tensor/src/low_level_processors/classify_output.rs:
##########


Review Comment:
   I dislike that one would have to scroll past 200-300 lines of boilerplate to 
get to the actual code, in my opinion this is a NiFi contract so they are not 
strictly related to the implementation so I would like to keep them separate, 
in trivial cases like in DrawBoundingBoxes with just a few properties sure we 
can keep them together, and anyway its still together only in a submodule, (we 
might even move the tests to a submodule, but im a bit more leniant on that at 
least thats usually at the end of the file), Ive looked at a bunch of high 
profile and its common to use extracted submodules, e.g. even in std the tests 
are in their separate files 
https://github.com/rust-lang/rust/blob/main/library/std/src/collections/hash/map.rs#L1-L2



##########
minifi_rust/extensions/minifi_tensor/src/low_level_processors/classify_output/classify_output_def.rs:
##########
@@ -0,0 +1,162 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use super::{ClassifyOutput, ScoreActivation};
+use minifi_native::{
+    OutputAttribute, ProcessorDefinition, ProcessorInputRequirement, Property, 
PropertyDefinition,
+    Relationship, property_definitions,
+};
+use std::path::PathBuf;
+

Review Comment:
   the c api still takes strings so we would have to do the conversion from T 
to string in const context, thats far from trivial unfortuently (afaik the rust 
support we would need for this is not in stable yet) we might be able to do it, 
or we might need to move from const Properties then its easy to implement



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to