-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add support for api-data.json file output #1427
Conversation
fixes apidoc#1043 fixes apidoc#1390 fixes apidoc#1046
Hello, Yes this can be interesting, but I would rather see a boolean flag, and the output is in the assets folder. |
Can you have a look now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, your code doesn't work because of capitalization bug of the option name in writer.js, but that's an easy fix.
Second, I would name the option: --write-json
instead. The idea is to remove the terminology "data" which means nothing and is a word that should be avoided.
Finally, it would be important to explain better what it is. Maybe in the help add something like: this will create a file "parsed.json" with the parsed API info collected by the parser and used as source for the generation of the html.
Finally finally, add a test that at least verifies that with this option, a file is created.
something like this? Still missing test. Which file should I add the test to?
Thank you for your contribution! |
@NicolasCARPi thanks for good and fast guidance |
When is the next scheduled release? Eagerly awaiting a release with my fix so I can update :) |
@gemal There are no schedule. If you want a release you'll need to make a PR that fixes a bug ;) (yeah, I'm blackmailing you!!) |
I know that this was just a feature but still :) |
not 100% sure I understand? I thought I fixed a bug or did that only qualify as a feature or how does it work? |
You added a feature, it is not a bugfix in my eyes, it was possible to get the data already, you just made it more convenient. Anyway, I'll make a release now. |
fixes #1043
fixes #1390
fixes #1046
Make sure to read the contributing documentation.
IMPORTANT: Base your PR off the 'dev' branch and target that branch for merging.
Would you accept a pull request like this? More work is needed but didn't want to do more if this was not an accepted way forward