-
Notifications
You must be signed in to change notification settings - Fork 19
Conversation
package.json
Outdated
@@ -128,7 +128,8 @@ | |||
"test": "lerna run test", | |||
"test:commit-check": "npm run doc:lint && npm run lint && npm run depcheck && npm run test", | |||
"test:security": "cd packages/udaru-hapi-server && npm run test:security", | |||
"swagger-gen": "node scripts/getSwaggerJson.js > docs/swagger/swagger.json && swagger-gen docs/swagger/swagger.json -d docs/swagger && node scripts/injectUdaruSwaggerCss.js" | |||
"swagger-gen": "node scripts/getSwaggerJson.js > docs/swagger/swagger.json && swagger-gen docs/swagger/swagger.json -d docs/swagger && node scripts/injectUdaruSwaggerCss.js", | |||
"update-swagger": "node scripts/getSwaggerJson.js js > docs/swagger/swagger-json.js" |
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.
where/when is this update-swagger task used now? Do we need to update the 'how to release' docs in contributing.md?
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.
It should probably replace swagger-gen, if swagger-gen is not replacing the script block then essentially it is not working, so suggest we replace the swagger-gen command with this command and remove existing?
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.
i think we should go with one approach or the other but not both.
In the https://github.com/nearform/trail repository I followed a different approach. The Swagger JSON file is generated on the fly, so we don't have to remember about scripts and similar. Since the parsing is only done once on startup, there's no performance overhead (rather than keeping the small JSON file in memory). Hope this helps! |
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.
LGTM
I put swagger-ui-dist 3.14.1 (latest), into a subfolder of docs/swagger so that the distribution remains intact, if upgrading in future, this folder can be replaced wholesale... Specific files we need such as the css, swagger-json.js and index.htm are in the root of docs/swagger For future updates to docs this will keep the process of upgrading simple and stable. swagger-gen npm command simply creates a swagger-json.js file, which index loads as per previous swagger-gen package. |
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.
LGTM
* constraints added to db tables and validation to check entity ids... * enforcing hyphenated ids, paths translated to underscore * tests to check for invalid id characters across entities * Fix for udaru bin (#530) * node 10 check (#533) * update of swagger json (#532) * update of swagger json * latest swagger-ui-dist * swagger-gen removed * policy assignment must use objects (#535) * policy assignment must use objects * label update for swagger docs * deleting for merge * merge fix * Merge swagger and validation (#536) * merge joi swagger with validation into core * constraints added to db tables and validation to check entity ids... * enforcing hyphenated ids, paths translated to underscore * master merge * no longer generated * removed after rebase * no longer user to inject (static in docs/swagger/index.htm)
The only item required to updated when updating swagger docs is that we update the swagger-json.js file.
For some reason the script section that swagger-gen replaces stopped working recently so it is probably a safer approach to just update the json and js files when performing an update. This should probably form part of every commit.
We can probably remove swagger-gen at this point as currently it is not working.
I updated the getSwagger.json to take an optional command like parameter js to generate json and assign to variable, without it, it works as previously and just outputs the json stringified.