search

OpenCyphal / nunavut

Generate code from DSDL using PyDSDL and Jinja2
https://nunavut.readthedocs.io/
Other
39 stars 24 forks source link

Issue #188 fix #204

Closed thirtytwobits closed 3 years ago

thirtytwobits commented 3 years ago

Completely refactored stropping. It was a bit of a mess but now that we have a couple languages in use it was easier to see the right design.

thirtytwobits commented 3 years ago

Okay, I just need to experiment with Black and I'll have responded to all your comments.

thirtytwobits commented 3 years ago

@pavel-kirienko . I believe I've addressed everything? Just waiting for your approval.

thirtytwobits commented 3 years ago

Oh wow. Something weird happened with my workspaces and changes I thought I had I missed. I should have committed everything now.

pavel-kirienko commented 3 years ago

I have no other comments here.

thirtytwobits commented 3 years ago

I changed back to pyyaml since the other library, apparently, would require users to install additional dependencies on Linux. Please re-review and approve.

pavel-kirienko commented 3 years ago

Can you expand on this a bit? Why do you think it requires additional dependencies?

On Sat, Jul 31, 2021, 05:38 Scott Dixon @.***> wrote:

I changed back to pyyaml since the other library, apparently, would require users to install additional dependencies on Linux. Please re-review and approve.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/UAVCAN/nunavut/pull/204#issuecomment-890279639, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZFIZFIE7DGD33JQ7AULQLT2NOZHANCNFSM5A3IDTNQ .

thirtytwobits commented 3 years ago

https://buildkite.com/uavcan/nunavut-pr/builds/417#80433505-7b0c-4ea5-bdc6-ecf5938fc30d

thirtytwobits commented 3 years ago

https://stackoverflow.com/questions/68626524/ruamel-yaml-requires-python-dev-in-ubuntu-container-build

thirtytwobits commented 3 years ago

So if we can agree on the same ruamel version for Yakut and Nunavut then I'll switch to ruamel. What say ye?

pavel-kirienko commented 3 years ago

This is oddly more complicated than I imagined. Let's just use 0.17 and call it a day.

thirtytwobits commented 3 years ago

So...are we super happy with this dependency? The author is willing to break things, the library is painfully new, and there are no MyPy types available? Nothing about using this over pyyaml is making me happy.

pavel-kirienko commented 3 years ago

PyYAML it is then.

thirtytwobits commented 3 years ago

Then the current PR stands as-is. Can I merge?

pavel-kirienko commented 3 years ago

Sure.

On Tue, Aug 3, 2021, 21:52 Scott Dixon @.***> wrote:

Then the current PR stands as-is. Can I merge?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/UAVCAN/nunavut/pull/204#issuecomment-892083052, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZFIZBJJR2C4CPQZOK5ID3T3A3FVANCNFSM5A3IDTNQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .