search

knurling-rs / flip-link

Adds zero-cost stack overflow protection to your embedded programs
Apache License 2.0
270 stars 6 forks source link

Feature: `+` supported in ORIGIN #70

Closed Dajamante closed 2 years ago

Dajamante commented 2 years ago

flip-link does now accept additions for Origin as in this issue. fixes #65.

Dajamante commented 2 years ago

Thank you for your work @Dajamante! It looks very good overall, there are just a few nitpicks.

Regarding the println statements you added: I understand that you added them for debugging purposes, which is a good thing to have. But I don't think they should appear in the normal output, since, as far as I understand, they are not meant for the user of flip-link, but rather a developer working on flip-link. Therefore please change them to log::debug! (or another log-level if you think it is better suited), since then they won't appear by default, but can be enabled via the RUST_LOG environment variable.

I am really sorry about the println!() statements, those should have been removed as you pointed out... I will address the rest of your comments shortly!

Dajamante commented 2 years ago

Thank you for addressing my review! I added two more, small, questions regarding the doc-comments.

No really, kudos to you 😄 , that was an excellent review! I addressed your two comments.

bors[bot] commented 2 years ago

:v: Dajamante can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

Dajamante commented 2 years ago

v Dajamante can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

bors r+

bors[bot] commented 2 years ago

Build succeeded: