refactor(let): Cleaned up the exec ecu code a lot#184
refactor(let): Cleaned up the exec ecu code a lot#184DanielVoogsgerd wants to merge 2 commits intoBraneFramework:mainfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /// A new map with the environment on success, or a LetError on failure. | ||
| fn construct_envs(variables: &Map<FullValue>) -> Result<Map<String>, LetError> { | ||
| // Simply add the values one-by-one | ||
| // FIXME: Use iterators |
Check notice
Code scanning / devskim
A "TODO" or similar was left in source code, possibly indicating incomplete functionality
| /// * `function`: The function name to execute in the package. | ||
| /// * `arguments`: The arguments, as a map of argument name / value pairs. | ||
| /// * `working_dir`: The wokring directory for this package. | ||
| /// Arguments: |
There was a problem hiding this comment.
Nowadays I'm using # Arguments everywhere, which plays nice with rustdoc. But not work blocking this PR on IMO
There was a problem hiding this comment.
I will fix it, I really wish Rust would take a stance on what is prefered, but while there was some sort of convention in the past, that convention has deliberately been removed from the documentation.
|
I've added fixes for 3/4 of yours FIXMEs ~ feel free to provide thoughts :) I'm unable to test if it works, though. I don't have access to a VM that compiles Longer-term fix: bring back in-container compilation. Workaround: #191 and update the Dockerfile to an Ubuntu version that is more reasonable to compile to. |
|
I think we can compile it using the new Makefile, however I am more limited by my broken Brane configuration which I am still trying to figure out. I will take a look once that is fixed (more important than this PR anyway). Thanks for the fixes, though! |
DanielVoogsgerd
force-pushed
the
let-clean
branch
from
5105635 to
b1e82c1
Compare
- Remove old commented code - Use more rusty language features (more consise)
Test pending, need to try out on a VM to get `branelet` container compatability lol
DanielVoogsgerd
force-pushed
the
let-clean
branch
from
b1e82c1 to
a2424d4
Compare
2 participants
I needed to clean this up a bit to make it easier to follow. I could refactor more, but I have to get back to other stuff now.