CLI

[kalinchernev]

In relation to #24

It's re-using the main module 100%, only accepts input from user and validates the required.
Example definition file and documentation are included.

I'm open for feedback :)

[chdanielmueller]

Hi @drGrove,
Would you like to take a look at this?

[kalinchernev]

Hi @chdanielmueller when I click on the codacy check details I can't see any details as pointed out from the line-by-line part. When I click on any of the individual links they lead me to my master and the errors are not in the report.
Could you please help me on how to get these details to be able to fix them?
Thanks!

[chdanielmueller]

Hi @kalinchernev
Sorry for the late answer. Has been a busy time. I hope to take a look at this on friday.
Please ignore the codacy checks as I am removing this service.
Thanks!

[drGrove]

@chdanielmueller will pull tonight and review. Sorry for the late response.

[kalinchernev]

Hi chdanielmueller, drGrove,
I was anyway offline last week, so no worries for delays, no hurry.
Thanks for your feedback when you have the opportunity!

[chdanielmueller]

Hi @drGrove,
How is your review going?

[drGrove]

Linked it globally and ran it against one of my rather large projects and it held up well.

I like the support for multiple globs, which make it nice for a folder heavy project where everything is split up and with the different types of definitions.

This kind of setup does require that the swaggerDef is not nested within a the swagger init call (which isn't the end of the world but we should make sure that people know that).

I would like to see some more test cases. None of my project have any real security definitions (yet), so i couldn't test that. But all in all this is really great 😄.

@kalinchernev, mind adding some more tests to make sure this is fully tested?

Side note: Swagger supports yaml input/output, should we as well?

Here's a couple of tools for yaml parsing:

• yaml2json
• json2yaml

Side note 2: If you were to include the whole project it would run it (which is also not ideal for people have have any definitions in the root file).

[kalinchernev]

Hi @drGrove and thanks for the review!

This kind of setup does require that the swaggerDef is not nested within a the swagger init call (which isn't the end of the world but we should make sure that people know that).

Could you please elaborate a bit, I don't understand it.

@kalinchernev, mind adding some more tests to make sure this is fully tested?

Yes, sure! I started writing some but the way the tests are setup is not easy to get into the flow. I'll spend time on this.

Side note: Swagger supports yaml input/output, should we as well?

Do you mean that we should also forsee parging yml files? I thought the module aims for parsing jsDoc comments from .js files?

[drGrove]

Side note: Swagger supports yaml input/output, should we as well?
Do you mean that we should also forsee parging yml files? I thought the module aims for parsing > jsDoc comments from .js files?

You're right, ignore me there

This kind of setup does require that the swaggerDef is not nested within a the swagger init call > > (which isn't the end of the world but we should make sure that people know that).
Could you please elaborate a bit, I don't understand it.

I wrote a yo generator for koa apis, in this i nest the swaggerDef into the index.js. If i were to include this as the swaggerdef, it would try and run the file (since it's been required). Basically what I mean is that the swaggerDef needs to be more stand-alone (which is fine), i'll actually adjust my project for this (since it more modular that way).

[kalinchernev]

@drGrove thanks for the examples. I have to be honest that it takes me some time to understand it, but theoretically the solution could be considered somewhere in the instantiation phase to check if an existing configuration is present or not. I'll have to dig into that further.

[kalinchernev]

@drGrove @chdanielmueller I added tests for the checks and scenarios of the application, please tell me if they are ok?

[drGrove]

@chdanielmueller I think this is ready for merge

[chdanielmueller]

@drGrove I'm also fine with this. Thank you very much for taking a look at it.

Would you like to merge it yourself, bump the version number to v1.4.0, create a new GitHub Tag and push it to npmjs?

[kalinchernev]

@drGrove @chdanielmueller thanks for the heads up!
I've just updated the version for the release :)

[kalinchernev]

Thanks @drGrove!