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

feat: custom Spectral rule to ensure content objects contain schema #258

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

barrett-schonefeld
Copy link
Contributor

Purpose:

  • Establish a pattern in the validator for adding custom spectral rules and custom spectral functions
  • Ensure content objects contain a schema

Changes:

  • Add custom spectral rule to ensure that content objects contain a schema
  • Add directory to store custom spectral rules and functions
  • Add code to retrieve and merge custom spectral rules

Tests:

  • Add tests for each use case outlined in the spectral rule

Copy link
Contributor

@mkistler mkistler left a comment

Choose a reason for hiding this comment

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

This does seem to satisfy the requirement but I think it can be simplified while still achieving the same result.

@barrett-schonefeld barrett-schonefeld force-pushed the content-entry-contains-schema branch 2 times, most recently from 57ea363 to 1943653 Compare March 9, 2021 16:39
@barrett-schonefeld barrett-schonefeld force-pushed the content-entry-contains-schema branch 2 times, most recently from 3bc8236 to 36ad0f4 Compare March 9, 2021 19:48
Copy link
Member

@dpopp07 dpopp07 left a comment

Choose a reason for hiding this comment

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

I think this looks good but I did have one question about the Spectral implementation to check on before merging (might want to wait for other reviews as well)

src/spectral/rulesets/.defaultsForSpectral.yaml Outdated Show resolved Hide resolved
@barrett-schonefeld barrett-schonefeld force-pushed the content-entry-contains-schema branch 2 times, most recently from 73453b8 to 0b09c2a Compare March 10, 2021 21:25
Copy link
Contributor

@mkistler mkistler left a comment

Choose a reason for hiding this comment

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

The code here looks good but another key part of creating our own custom rules is documenting them. We probably need to add a new section to the README that describes each custom rule, like the docs/reference/openapi-rules.md in Spectral.

@barrett-schonefeld
Copy link
Contributor Author

The code here looks good but another key part of creating our own custom rules is documenting them. We probably need to add a new section to the README that describes each custom rule, like the docs/reference/openapi-rules.md in Spectral.

To keep the README.md from getting overcrowded with rule descriptions, I documented the rule in docs/spectral-rules.md and linked to the rule descriptions in the README.MD.

Copy link
Contributor

@mkistler mkistler left a comment

Choose a reason for hiding this comment

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

Almost done, but we need to document this from the point of view of the user.

README.md Outdated Show resolved Hide resolved
docs/spectral-rules.md Outdated Show resolved Hide resolved
Purpose:
- Establish a pattern in the validator for adding custom spectral rules and custom spectral functions
- Ensure content objects contain a schema

Changes:
- Add custom spectral rule to ensure that content objects contain a schema (defaults to warning)

Tests:
- Add tests for each use case outlined in the spectral rule

Docs:
- Add a central location for documenting custom Spectral rules
- Document the `content-entry-contains-schema` rule
Copy link
Contributor

@mkistler mkistler 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. 👍

@barrett-schonefeld barrett-schonefeld merged commit bb9e419 into main Mar 16, 2021
dpopp07 pushed a commit that referenced this pull request Mar 16, 2021
# [0.37.0](v0.36.0...v0.37.0) (2021-03-16)

### Features

* custom Spectral rule to ensure content objects contain schema ([#258](#258)) ([bb9e419](bb9e419))
@dpopp07
Copy link
Member

dpopp07 commented Mar 16, 2021

🎉 This PR is included in version 0.37.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@barrett-schonefeld barrett-schonefeld deleted the content-entry-contains-schema branch March 16, 2021 13:56
This was referenced Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants