Skip to content
This repository was archived by the owner on Apr 8, 2024. It is now read-only.

Fix/ven 54 fix documentation values#53

Open
Neozaru wants to merge 10 commits intomasterfrom
fix/VEN-54-fixDocumentationValues
Open

Fix/ven 54 fix documentation values#53
Neozaru wants to merge 10 commits intomasterfrom
fix/VEN-54-fixDocumentationValues

Conversation

@Neozaru
Copy link
Contributor

@Neozaru Neozaru commented Aug 13, 2021

Copy link
Contributor

@arijoon arijoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'm guessing we just copy/paste the swagger.json over the v12 file anytime we have an API update and all should work as expected?

"dependencies": {
"@babel/polyfill": "^7.8.3",
"highlight-words-core": "^1.2.2",
"lodash.mapvalues": "^4.6.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might as well include the entire lodash

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was doing this before noticing that we've done this type of fine grained dep injection so I figured out there might be a reason although I am not sure. Do you think the weight different will be negligible if we import everything ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, especially in our usecase a few more KBs of lodash won't really have a major effect

getHeadersWithExtras(parameters.filter(parameter => parameter.in === 'header'))
).map(([key, value]) => ` "${key}": "${value}"`)
.join(',\n') + '\n' +
// ' "Accept": "application/json",\n' +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be removed or is it needed in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'm guessing we just copy/paste the swagger.json over the v12 file anytime we have an API update and all should work as expected?

Correct. I'll add something in Confluence to explain the process, but it should be globally unchanged. The goal is to make doc updates as easy as possible so it can be updated more or less at the same time

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a really nice step if we let this automatically fetch the swagger.json file from our API and only provide the overlay, this will make sure it's always in sync

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in latest commits ;)

@Neozaru Neozaru force-pushed the fix/VEN-54-fixDocumentationValues branch from 8eb1894 to cd3f36d Compare September 14, 2021 14:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants