add base support for exporting objects and classes in node_loader#343
add base support for exporting objects and classes in node_loader#343rxbryan wants to merge 12 commits intometacall:developfrom
Conversation
1a5663b to
53be583
Compare
6fb9758 to
1334267
Compare
00bc5a8 to
22e43f3
Compare
2678438 to
67e344f
Compare
|
Last commits look pretty well, I recommend you to implement tests in order to verify the behavior. TDD is always easier to develop. Check out |
|
Check out the tests, no one is passing, try to pass them before pushing. |
e44e0ab to
f6078f4
Compare
I just figured out what's making the tests fail. I would fix it as soon as possible |
add api for getting all keys and values in adt_set and adt_map
|
I node port it seems there's a bug:
|
|
Also there's this in windows:
But I think this has been already solved in develop, can you merge develop into your branch and push so we can see if that test passes? |
My branch is up-to-date. I don't run windows so I can't really debug the windows test |
That's not the only problem. There's alot of load_from_file errors... look at this Where should |
Those errors are fine. I am reviewing everything, you overwrote all my changes. I am going to try to solve them. |
|
I think I almost get it to work, I want still to fully review it locally. |
|
Yeah, it seems all tests passed, but I still want to fully review it. Formatting failed because I modified it from the web, but I also want to fully review all the implementation properly, I've seen things that are not clear to me and I need to understand them better and provide better feedback to you. Good job! |
|
@rxbryan hey mate I am going to review all this and probably reimplement it using parts of your PR, I prefer doing that because it's the most effective way of reviewing it in depth, specially for debugging complex memory issues or similar. Once I finish it I will co-author the PR with you so you get credits of the task. This is the last step towards making NodeJS fully interoperable between Python and Ruby. |
|
Hi @viferga
Iooking forward to that. |
Description
add base support for exporting objects and classes in node_loader
Fixes #(issue_no)
Type of change
Checklist:
make testorctest -VV -R <test-name>)../docker-compose.sh build &> outputand attached the output.OPTION_BUILD_SANITIZERor./docker-compose.sh test &> outputandOPTION_TEST_MEMORYCHECK.Helgrindin case my code works with threading.make clang-formatin order to format my code and my code follows the style guidelines.If you are unclear about any of the above checks, have a look at our documentation here.