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(core): support multiple JSONPath expressions for aliases #2016

Merged
merged 8 commits into from
Jan 19, 2022
Merged

feat(core): support multiple JSONPath expressions for aliases #2016

merged 8 commits into from
Jan 19, 2022

Conversation

P0lip
Copy link
Contributor

@P0lip P0lip commented Jan 4, 2022 •
edited

Related to https://github.com/stoplightio/platform-internal/issues/8141

Needed by #2018
Example usage

spectral/packages/rulesets/src/oas/index.ts

Line 38 in a2019f1

{ formats: [oas2], given: ['$.responses', '#OperationObject.responses'] },

I believe that once this one is in, we can consider the aliases frozen and won't need to make any further changes.

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 4, 2022
@P0lip P0lip self-assigned this Jan 4, 2022
P0lip
P0lip commented Jan 4, 2022
View reviewed changes
docs/guides/4-custom-rulesets.md
@@ -322,13 +324,69 @@ For now the JSON, YAML, and JS, are all being maintained, and there are no curre

## Aliases
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mnaumanali94 what do you think?

@P0lip P0lip force-pushed the feat/core/multiple-aliases branch from bf3ae2b to b397d16 Compare January 5, 2022 23:11
@P0lip P0lip mentioned this pull request Jan 6, 2022
Closed
4 tasks
@P0lip P0lip marked this pull request as ready for review January 6, 2022 16:06
marbemac
marbemac reviewed Jan 7, 2022
View reviewed changes
docs/guides/4-custom-rulesets.md Outdated
This is the JSON Schema definition describing aliases.
It's a superset of `given`, with the notable difference being the possibility to distinguish between different formats.

```json
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this will really be all that helpful for folks reading the docs, and might just be confusing? I'm pretty well versed with json schema, and even so it's not easy to imagine what this schema is trying to describe. The examples are much more helpful 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, I might just delete it.
Do you think the examples here are sufficient or should I add a few additional ones?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think delete and good for now

docs/guides/4-custom-rulesets.md Show resolved Hide resolved
docs/guides/4-custom-rulesets.md Outdated Show resolved Hide resolved
packages/core/src/meta/ruleset.schema.json Show resolved Hide resolved
packages/core/src/ruleset/rule.ts Outdated Show resolved Hide resolved
marbemac
marbemac approved these changes Jan 18, 2022
View reviewed changes
Copy link
Contributor

@marbemac marbemac left a comment

Choose a reason for hiding this comment

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

Generally looks good - however as discussed, definitely think that we should not expose/advertise the baked in aliases until we have time to really think them through.

@P0lip P0lip merged commit f1b2c2c into develop Jan 19, 2022
@P0lip P0lip deleted the feat/core/multiple-aliases branch January 19, 2022 00:55
stoplight-bot pushed a commit that referenced this pull request Jan 19, 2022
@StoplightSupport
chore(release): 1.9.0 [skip ci]
92de929 
# [@stoplight/spectral-core-v1.9.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-core-v1.8.1...@stoplight/spectral-core-v1.9.0) (2022-01-19)

### Features

* **core:** support multiple JSONPath expressions for aliases ([#2016](#2016)) ([f1b2c2c](f1b2c2c))
@stoplight-bot
Copy link
Collaborator

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

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

Your semantic-release bot 📦🚀

@DiogoDoreto
Copy link

This PR introduced a breaking change to the aliases property. I started seeing errors like the one below after updating:

Error at #/aliases/PathItem: must be array

I'll update my code accordingly, but I'm adding this comment here to help anyone searching for this error in the future.

@P0lip
Copy link
Contributor Author

P0lip commented Feb 24, 2022

@DiogoDoreto true. I felt a bit uncomfortable releasing this change as non-breaking, as I knew this could affect a few users using it. In fact, the initial iteration of this PR was backward compatible, but having a few internal conversations we decided to make that soft breaking change.
However, rest assured it's a one-off situation, as we do try to strictly follow semantic versioning at all times.

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

@marbemac marbemac marbemac approved these changes

@mnaumanali94 mnaumanali94 Awaiting requested review from mnaumanali94

Assignees

@P0lip P0lip

Labels
enhancement New feature or request released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@P0lip @stoplight-bot @DiogoDoreto @marbemac