Import Web Assembly support from snapchat's djinni fork #157
Conversation
mutagene
force-pushed
the
import-wasm-support-from-snapchat
branch
from
9d18164
to
9ea2a7b
Compare
|
Wow, that's a pretty fantastic development! Just a question: After looking back at the Python addition, I see that it is now 80% to 90% done but then slowed down. |
mutagene
changed the title
|
Yes, I think we need some kind of definitive sample for this to be moved out of draft status. I haven't checked what there is in the snapchat repository, yet. I understand your concerns about abandoned platforms, but (at least for me) web assembly is pretty essential. I think that, given the state of the support on the snapchat fork, it should be possible to get the web assembly support here quite usable without too much pain. Fingers crossed. Next up the support lib, I guess. |
mutagene
force-pushed
the
import-wasm-support-from-snapchat
branch
3 times, most recently
from
3027080
to
e0c0dce
Compare
src/it/resources/expected/all_datatypes_json/cpp-headers/my_flags.hpp
Outdated
Show resolved
Hide resolved
mutagene
force-pushed
the
import-wasm-support-from-snapchat
branch
from
801ded6
to
76ef1b2
Compare
|
One of my general concerns with this PR is how many changes seem be outside of the new generator code, as in theory, adding a new generator should change little of the existing code other than extending the places to enable the new output. E.g. Ive started working on a swift and kotlin generator and so far haven't had to make any changes to existing code other than adding the options and extra bools to enable them during testing |
|
I agree, we need to check that all changes are related to the WASM part and not sneak in via any other Snapchat additions that might change behavior. On the other hand, if the expected files from the tests are unchanged, nothing should have changed (or our tests are incomplete). So we should have some safety net in place, |
I've been trying to reduce these changes. I think all that remains is the |
mutagene
force-pushed
the
import-wasm-support-from-snapchat
branch
from
d77aee4
to
d78420d
Compare
mutagene
changed the title
mutagene
changed the title
|
I am sorry, but looking at my calendar, it is very likely my review must wait until the Christmas holidays. |
mutagene
force-pushed
the
import-wasm-support-from-snapchat
branch
from
ba6c5bb
to
e5e0995
Compare
I felt quite inspired by @eakoli 's latest contribution. I'm not sure there's the appetite for this, but at least on my projects, if I'm using djinni in the future it's hard to imagine not wanting the possibility of a web assembly (and flutter?) target. This is quite a naive attempt to steal that from the snapchat djinni, and without the lib-side work (which I haven't done yet), I'm not sure it even works. Anyway, the way these things go I may have time to work on it some more and I may not, so I thought I'd make the (draft) PR now and then worry about the rest later.