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

Jump to bottom

feat(core): improve ruleset validation #2026

Merged
merged 2 commits into from Aug 3, 2022
Merged

feat(core): improve ruleset validation #2026

merged 2 commits into from Aug 3, 2022

Conversation

P0lip
Copy link
Contributor

@P0lip P0lip commented Jan 18, 2022 •
edited by philsturgeon

Fixes #2186

This will let us remove https://github.com/stoplightio/platform-internal/tree/f7fddbb1bfe6a760819a2568a7eed50732774af7/services/projects/src/utils/assertValidRuleset

A non-exhaustive list of changes:

  • Added JSON Schema definitions that are suitable for JSON/YAML rulesets - going to be useful internally, particularly in the case of the ruleset-migrator package. Due to that, assertValidRuleset accepts a second parameter, namely format.
  • Fixed the validation of function options.
  • Instead of a single error with all messages concatenated, we now throw an AggregateError to retain each validation error, and ultimately make the errors easier to consume. This resulted in a lot of test changes, as well as the addition of that custom expect matcher.
  • Moved JSON Schema definitions describing rulesets from src/meta to src/ruleset/meta
  • Setup package exports so it's easier to import the validator
  • FormatsSet has been renamed to Formats
  • Improve toJSON signatures -> 2358b85
  • Moved some files under src/ruleset around to unify the directory structure

Do note that although some changes are marked as additions, they are not such - it's mostly me slicing certain chunks of code into separate files. In other words, a notable deletion of code is usually followed by an addition somewhere.

Checklist

  • Tests added / updated
  • Docs added / updated

Does this PR introduce a breaking change?

  • Yes
  • No

@P0lip P0lip added the enhancement New feature or request label Jan 18, 2022
@P0lip P0lip self-assigned this Jan 18, 2022
@P0lip P0lip force-pushed the feat/core/validation-rework branch 4 times, most recently from 8069c55 to 088d42b Compare January 19, 2022 02:23
@P0lip P0lip changed the title feat(core): improve validation feat(core): improve ruleset validation Jan 19, 2022
@P0lip P0lip force-pushed the feat/core/validation-rework branch 2 times, most recently from 704bc83 to c0d5c70 Compare January 31, 2022 13:14
@P0lip P0lip force-pushed the feat/core/validation-rework branch from c0d5c70 to 2358b85 Compare February 14, 2022 18:52
@P0lip P0lip marked this pull request as ready for review February 14, 2022 19:15
@P0lip P0lip force-pushed the feat/core/validation-rework branch from 2358b85 to 1020bfc Compare February 14, 2022 19:15
@P0lip P0lip requested a review from mmiask February 14, 2022 19:16
@P0lip P0lip mentioned this pull request Feb 15, 2022
Merged
4 tasks
@Nezteb
Copy link

Nezteb commented Mar 2, 2022

@P0lip @mmiask This PR is pretty stale. Can it be closed? If not, what is its priority?

@mmiask
Copy link
Contributor

mmiask commented Mar 7, 2022

@P0lip @mmiask This PR is pretty stale. Can it be closed? If not, what is its priority?

@Nezteb IMHO this should be review ASAP, although I've been busy with sprint-related stuff. I'll take a look as soon as I can.

@P0lip
Copy link
Contributor Author

P0lip commented Mar 7, 2022

Let's review #2062 first. It's a subset of the following PR.

@P0lip P0lip marked this pull request as draft March 7, 2022 12:41
@pamgoodrich
Copy link
Contributor

@P0lip : Does this issue require Spectral doc updates and if so, would you like help?

@P0lip
Copy link
Contributor Author

P0lip commented Apr 14, 2022

Thanks @pamgoodrich, I think we're good. This mostly improves the existing functionality, so I expect no docs changes to be required, but I'll make sure to let you know in case something changes.

@P0lip P0lip force-pushed the feat/core/validation-rework branch from 1020bfc to 39708e9 Compare April 14, 2022 15:47
@P0lip P0lip mentioned this pull request Apr 26, 2022
Merged
4 tasks
@Nezteb Nezteb added team/dnc Team: Dazed and Confused and removed team/dnc Team: Dazed and Confused labels Apr 26, 2022
@P0lip P0lip force-pushed the feat/core/validation-rework branch from 39708e9 to e9fcea5 Compare April 27, 2022 12:44
@P0lip P0lip marked this pull request as ready for review April 27, 2022 12:51
@P0lip P0lip requested a review from a team as a code owner April 27, 2022 12:51
@P0lip P0lip requested a review from a team April 27, 2022 12:51
@P0lip P0lip force-pushed the feat/core/validation-rework branch from e9fcea5 to f53ee36 Compare April 27, 2022 12:53
@Nezteb Nezteb added team/dnc Team: Dazed and Confused and removed team/dnc Team: Dazed and Confused labels Apr 27, 2022
@Nezteb Nezteb added team/dnc Team: Dazed and Confused and removed team/dnc Team: Dazed and Confused labels May 12, 2022
@Nezteb Nezteb added the team/dnc Team: Dazed and Confused label May 24, 2022
@P0lip P0lip mentioned this pull request Jun 18, 2022
Closed
philsturgeon
philsturgeon previously approved these changes Jun 20, 2022
View reviewed changes
Copy link
Contributor

@philsturgeon philsturgeon left a comment

Choose a reason for hiding this comment

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

We know im not the best at the sort of super advanced TypeScript here, but I believe the tests speak for themselves, and I can trust @P0lip with the rest.

@philsturgeon philsturgeon mentioned this pull request Jul 6, 2022
Closed
mnaumanali94
mnaumanali94 previously approved these changes Aug 2, 2022
View reviewed changes
@P0lip P0lip dismissed stale reviews from mnaumanali94 and philsturgeon via 1329113 August 3, 2022 12:00
@P0lip P0lip merged commit 8315162 into develop Aug 3, 2022
@P0lip P0lip deleted the feat/core/validation-rework branch August 3, 2022 12:06
stoplight-bot pushed a commit that referenced this pull request Aug 3, 2022
@StoplightSupport
chore(release): 6.5.0 [skip ci]
739140f 
# [@stoplight/spectral-cli-v6.5.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-cli-v6.4.2...@stoplight/spectral-cli-v6.5.0) (2022-08-03)

### Features

* **core:** improve validation ([#2026](#2026)) ([8315162](8315162))
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version @stoplight/spectral-cli-v6.5.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

stoplight-bot pushed a commit that referenced this pull request Aug 3, 2022
@StoplightSupport
chore(release): 1.13.0 [skip ci]
91e1d73 
# [@stoplight/spectral-core-v1.13.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-core-v1.12.4...@stoplight/spectral-core-v1.13.0) (2022-08-03)

### Features

* **core:** improve validation ([#2026](#2026)) ([8315162](8315162))
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version @stoplight/spectral-core-v1.13.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

stoplight-bot pushed a commit that referenced this pull request Aug 3, 2022
@StoplightSupport
chore(release): 1.7.0 [skip ci]
729f8da 
# [@stoplight/spectral-functions-v1.7.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-functions-v1.6.2...@stoplight/spectral-functions-v1.7.0) (2022-08-03)

### Features

* **core:** improve validation ([#2026](#2026)) ([8315162](8315162))
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version @stoplight/spectral-functions-v1.7.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philsturgeon philsturgeon philsturgeon left review comments

@mnaumanali94 mnaumanali94 mnaumanali94 left review comments

@mmiask mmiask Awaiting requested review from mmiask

@mallachari mallachari Awaiting requested review from mallachari

@Amjcraft Amjcraft Awaiting requested review from Amjcraft

@dotslashderek dotslashderek Awaiting requested review from dotslashderek

@jasonmgillhub jasonmgillhub Awaiting requested review from jasonmgillhub

Assignees

@P0lip P0lip

Labels
enhancement New feature or request released team/dnc Team: Dazed and Confused
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Invalid ruleset gives Error [object Object]
7 participants
@P0lip @Nezteb @mmiask @pamgoodrich @stoplight-bot @philsturgeon @mnaumanali94