search

boschresearch / blech

Blech is a language for developing reactive, real-time critical embedded software.
Apache License 2.0
72 stars 5 forks source link

Generating printState function and trace calls only on --trace #19

Closed frameworklabs closed 4 years ago

frameworklabs commented 4 years ago

Generating printState function and trace calls in app only when --trace option is given.

FriedrichGretz commented 4 years ago

Congrats on being the first to post a PR :-) It looks good. We will get back to you with technical feedback shortly. Since we went online only recently we have lacked a contribution process. In fact a CLA based approach is currently under internal review. For the time being we resort to a simple DCO based process. Sorry about that... The details are in CONTRIBUTING.md. Essentially it says you should add yourself to the list of authors in NOTICE. And we can only accept commits that are signed with the "Signed-off-by ..." line. So when we agree that the PR is 100% fine, for example squash your commits into one that is signed.

FriedrichGretz commented 4 years ago

Now to the interesting bit :-)

Please add + " --trace " in file test/testCodegeneration.fs after line 276 to reproduce the original behaviour.

Apart from this, everything works fine. Great job!

And just a minor comment. I'd suggest to rewrite the <.> empty <.> construction in CodeGeneration.fs:381 as

[ Comment.cTraceFunction
  traceFunctionPrototype]
|> dpToplevel

to better express the intention.

I believe @schorg has another comment on dll versioning...

schorg commented 4 years ago

I will increase dll versioning after the merge.

Currently Version handling is just too distributed over files and repositories.

frameworklabs commented 4 years ago

I am currently waiting on my employer to get the go for contributing to this project. After that I plan to address issue #18

FriedrichGretz commented 4 years ago

... and it's merged. Thanks you very much for the contribution. Looking forward to more ;-)

frameworklabs commented 4 years ago

Thanks for merging!

Blech is a really exciting and profound project / technology - nice to be able to contribute to it.