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.

Sign up for GitHub

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: $ref cannot have siblings #473

Merged
merged 16 commits into from
Aug 27, 2019
Merged

feat: $ref cannot have siblings #473

merged 16 commits into from
Aug 27, 2019

Conversation

P0lip
Copy link
Contributor

@P0lip P0lip commented Aug 21, 2019 •
edited
Loading

Closes #394

Checklist

  • Tests added / updated
  • Docs added / updated

Does this PR introduce a breaking change?

  • Yes
  • No

It's not a rule, as rules have no access to original (unresolved) data.
I don't know if it's a big deal or not, but if we do need a rule, I can make it a rule.
The code is placed in a wrong place and an extra test wouldn't hurt, so it's still a WIP.

@P0lip P0lip added the enhancement New feature or request label Aug 21, 2019
@P0lip P0lip self-assigned this Aug 21, 2019
@philsturgeon philsturgeon mentioned this pull request Aug 22, 2019
Closed
@P0lip P0lip force-pushed the feat/no-ref-siblings branch from 078fe3e to 2f878bc Compare August 22, 2019 19:04
@P0lip
Copy link
Contributor Author

P0lip commented Aug 22, 2019

@philsturgeon

It's not a rule, as rules have no access to original (unresolved) data.
I don't know if it's a big deal or not, but if we do need a rule, I can make it a rule.

How do you feel about not making it a rule and failing all the time same as invalid-ref does?

philsturgeon
philsturgeon reviewed Aug 23, 2019
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.

This will probably need to be a rule if possible because JSON Schema draft 8 will allow siblings and that means OpenAPI v3.1 might allow siblings and we'll need to keep this out of spectral itself.

@P0lip
Copy link
Contributor Author

P0lip commented Aug 23, 2019

@philsturgeon got it, will make it a rule then.

@P0lip
Copy link
Contributor Author

P0lip commented Aug 23, 2019

@philsturgeon
I'm planning to add an extra property to a rule named unresolved or something, which would make each rule operate on initial data

@nulltoken nulltoken mentioned this pull request Aug 24, 2019
Closed
@P0lip P0lip force-pushed the feat/no-ref-siblings branch from c9fbc48 to 46fe6e1 Compare August 25, 2019 10:10
@P0lip P0lip requested review from marbemac and philsturgeon August 26, 2019 12:30
@P0lip P0lip marked this pull request as ready for review August 26, 2019 12:30
@P0lip
Copy link
Contributor Author

P0lip commented Aug 26, 2019 •
edited
Loading

Hold on with the review, forgot to add one test.

Good to go!

@P0lip P0lip force-pushed the feat/no-ref-siblings branch from 80650fd to 61939f5 Compare August 26, 2019 21:27
@P0lip P0lip added this to the Aug '19 Hardening milestone Aug 26, 2019
philsturgeon
philsturgeon reviewed Aug 27, 2019
View reviewed changes
src/types/rule.ts Outdated Show resolved Hide resolved
@P0lip P0lip requested a review from philsturgeon August 27, 2019 12:14
philsturgeon
philsturgeon reviewed Aug 27, 2019
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.

When that variable is renamed we are all set.

@P0lip P0lip requested a review from philsturgeon August 27, 2019 13:50
@philsturgeon philsturgeon merged commit 2a85491 into develop Aug 27, 2019
@philsturgeon philsturgeon deleted the feat/no-ref-siblings branch August 27, 2019 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philsturgeon philsturgeon philsturgeon approved these changes

@marbemac marbemac Awaiting requested review from marbemac

Assignees

@P0lip P0lip

Labels
enhancement New feature or request
Projects
None yet
Milestone
Aug '19 Hardening
Development

Successfully merging this pull request may close these issues.

oas core rule: $ref cannot have siblings
2 participants
@P0lip @philsturgeon