-
-
Notifications
You must be signed in to change notification settings - Fork 248
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(ruleset-bundler): add ruleset loader #1988
Conversation
61c7bad
to
5b0cf6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excellent work, mostly just a couple of questions
.circleci/config.yml
Outdated
@@ -72,7 +72,7 @@ commands: | |||
- run: yarn pretest | |||
- run: | |||
name: Run node tests | |||
command: yarn test.jest --coverage --maxWorkers=<< parameters.max-workers >> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This removal of --coverage
intentional? (ok if it is, just double checking)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's already gone in develop branch
packages/ruleset-bundler/README.md
Outdated
|
||
## Loading YAML/JSON Ruleset | ||
|
||
It is assumed you have [@stoplight/spectral-ruleset-migrator](https://www.npmjs.com/package/@stoplight/spectral-ruleset-migrator) installed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to not have this as a dependency if it's always required? Env issue, dedupe issue, something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason to not have this as a dependency if it's always required? Env issue, dedupe issue, something else?
hmmm, yeah, we could have it installed, why not.
"default": "./dist/loader/node.js", | ||
"node": "./dist/loader/node.js", | ||
"browser": "./dist/loader/browser.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
export async function bundle(rulesetFile: string, bundleOptions: BundleOptions, { fs }: IO): Promise<string> { | ||
try { | ||
if (isBasicRuleset(rulesetFile)) { | ||
const migratedRuleset = await migrateRuleset(rulesetFile, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I think I see - migrateRuleset
naming confused me. More like a ruleset transformer in this case, to transform from plain object format to esm format.
# [@stoplight/spectral-ruleset-bundler-v1.1.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-ruleset-bundler-v1.0.1...@stoplight/spectral-ruleset-bundler-v1.1.0) (2021-12-15) ### Features * **ruleset-bundler:** add ruleset loader ([#1988](#1988)) ([a2c84dc](a2c84dc))
# [@stoplight/spectral-ruleset-migrator-v1.6.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-ruleset-migrator-v1.5.2...@stoplight/spectral-ruleset-migrator-v1.6.0) (2021-12-15) ### Features * **ruleset-bundler:** add ruleset loader ([#1988](#1988)) ([a2c84dc](a2c84dc))
# [@stoplight/spectral-ruleset-bundler-v1.1.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-ruleset-bundler-v1.0.1...@stoplight/spectral-ruleset-bundler-v1.1.0) (2021-12-15) ### Features * **ruleset-bundler:** add ruleset loader ([#1988](#1988)) ([a2c84dc](a2c84dc))
# [@stoplight/spectral-ruleset-migrator-v1.6.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-ruleset-migrator-v1.5.2...@stoplight/spectral-ruleset-migrator-v1.6.0) (2021-12-15) ### Features * **ruleset-bundler:** add ruleset loader ([#1988](#1988)) ([a2c84dc](a2c84dc))
# [@stoplight/spectral-ruleset-bundler-v1.1.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-ruleset-bundler-v1.0.1...@stoplight/spectral-ruleset-bundler-v1.1.0) (2021-12-15) ### Features * **ruleset-bundler:** add ruleset loader ([#1988](#1988)) ([a2c84dc](a2c84dc))
🎉 This PR is included in version @stoplight/spectral-ruleset-bundler-v1.1.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
# [@stoplight/spectral-ruleset-migrator-v1.6.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-ruleset-migrator-v1.5.2...@stoplight/spectral-ruleset-migrator-v1.6.0) (2021-12-15) ### Features * **ruleset-bundler:** add ruleset loader ([#1988](#1988)) ([a2c84dc](a2c84dc))
🎉 This PR is included in version @stoplight/spectral-ruleset-migrator-v1.6.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
Closes #1956
Checklist
Does this PR introduce a breaking change?