Skip to content

Conversation

@knolleary
Copy link
Member

Proposed changes

Ahead of adding support for subflow modules to NodeGen, I have gone through the code and done a bit of restructuring that should make it easier to maintain in the future.

Rather than have all of the generators in the one file, I have split them into their own modules - and moved the tests to match.

I have also moved much of the type-specific logic from the top-level node-red-nodegen.js file, down into the individual modules. That will make it easier to reuse as a library.

I have also completed some chores:

  • removed the deprecated request library in favour of axios
  • removed when as we can rely on native Promise support
  • removed istanbul as it cannot handle ES2015+ code (ie async/await)
  • added swagger.json to samples and removed the task that downloaded it from the internet every time the tests ran
  • added a grunt test task to generate a module from MyLampThing.jsonld

There are some further tasks I would like to look at:

  • update the Function generator to support the latest NR features (setup/close function blocks)
  • update dependency versions in general - we're back level on quite a few
  • document the APIs

Running npm install generates a node_modules that is over 600Mb in size. I know some of that will be devDependencies, but I'd really like to reduce the footprint of the library. jimp seems to be to blame for 40Mb of that - I realise that's used to handle icons, but it does feel like there should be a way to do that with a smaller footprint.

I see there are no tests for MyLampThing.jsonld - as it stands, we have no way of knowing if the WoT output works. I don't know enough about how it is meant to work to write the tests... we need to be very careful about adding support for new things with the tests in place for them from the beginning.

@knolleary
Copy link
Member Author

I see now the Travis build is failing on the Swagger node. Oddly, this was failing for me as well in the existing code and only started working when I changed it to use samples/swagger.json rather than download it each time.

Looking more closely at Travis, I see its doing a lot of stuff with docker and swaggerapi/petstore - it looks like it sets up a fake version of http://petstore.swagger.io - so the download happens from the docker container, not the internet.

Putting aside whether that's a good thing or not, need to figure out why the travis build fails with the local swagger file.

@kazuhitoyokoi
Copy link
Member

@knolleary I surveyed and fixed the Travis error. Can I add my commit in the #117 to this pull request?

@knolleary
Copy link
Member Author

@kazuhitoyokoi I've cherry-picked the commit from #117 into this PR and the tests are now passing cleanly. Thank you!

If you are happy with my proposed refactor, please do merge this PR.

@kazuhitoyokoi
Copy link
Member

@knolleary Thank you! I like this code structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants