Skip to content
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

Replace speccy with redocly #810

Closed
wants to merge 1 commit into from
Closed

Conversation

nhsd-david-wass
Copy link
Contributor

Summary

  • Routine Change
  • ❗ Breaking Change
  • 🤖 Operational or Infrastructure Change
  • ✨ New Feature
  • ⚠️ Potential issues that might be caused by this change

Add any other relevant notes or explanations here. Remove this line if you have nothing to add.

Reviews Required

  • Dev
  • Test
  • Tech Author
  • Product Owner

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

  • I have reviewed the changes in this PR and they fill all or part of the acceptance criteria of the ticket, and the code is in a mergeable state.
  • If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
  • I have ensured the changelog has been updated by the submitter, if necessary.

@nhsd-david-wass nhsd-david-wass force-pushed the feature/ERSSUP-64283 branch 2 times, most recently from 7b46ff3 to 83b2152 Compare May 2, 2023 13:37
@@ -14,6 +14,6 @@ content:
$ref: '../../eRS-ReferralRequest.yaml'
examples:
withClinicalInformation:
summary: `ReferralRequest` with clinical information (2 files)
summary: "`ReferralRequest` with clinical information (2 files)"
value:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Caused error starting the field with "`" - surrounding with " does not affect the rendering in bloomreach

@@ -11,7 +11,7 @@ content:
schema:
$ref: '../../eRS-FetchWorklist-List.yaml'
examples:
a&g-requests:
aAndg-requests:
summary: Advice and guidance requests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

'&' cause linting error - this field is not displayed - so does not affect output

redocly.yaml Outdated
rules:
no-invalid-media-type-examples:
severity: off
info-license-url: off
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no-invalid-media-type-examples is a linting rule that should check all the examples against the schema specification.
Where we have refs to example response files, the redocly library fails to expand the ref and fails the validation (known issue Redocly/redocly-cli#919)

However, we have our own validation (validate_oas_examples.py) that validates all our examples against the schema

Copy link
Contributor

@nhsd-jack-wainwright nhsd-jack-wainwright 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 to me 🙂, just a couple very minor comments.

package.json Outdated
@@ -3,16 +3,16 @@
"version": "0.0.1",
"description": "OAS (Swagger v3) API Definition for Template API",
"scripts": {
"lint-oas": "node_modules/.bin/speccy lint -s openapi-tags -s operation-tags specification/e-referrals-service-api.yaml --skip default-and-example-are-redundant",
"publish": "mkdir -p build && node_modules/.bin/speccy resolve specification/e-referrals-service-api.yaml -i | poetry run python scripts/yaml2json.py | poetry run python scripts/set_version.py | poetry run python scripts/populate_placeholders.py > build/e-referrals-service-api.json",
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that the yaml2json file has been removed from this command I suspect it can now be deleted as I don't think it's used elsewhere.

package.json Outdated
"check-licenses": "node_modules/.bin/license-checker --failOn GPL --failOn LGPL"
},
"author": "NHS Digital",
"license": "MIT",
"homepage": "https://github.com/NHSDigital/e-referrals-service-api",
"dependencies": {
"speccy": "^0.11.0"
"@redocly/cli": "^1.0.0-beta.124"
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a bit dangerous to depend on a beta release here (although we were depending on a 0.x version of speccy anyway), is there a non-beta release we could use instead?

Choose a reason for hiding this comment

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

@nhsd-david-wass nhsd-david-wass deleted the feature/ERSSUP-64283 branch June 6, 2023 07:59
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