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

docs(repo): update spectral in javascript examples #2181

Merged
merged 12 commits into from
Jun 28, 2022
Merged

docs(repo): update spectral in javascript examples #2181

merged 12 commits into from
Jun 28, 2022

Conversation

heitortsergent
Copy link
Contributor

@heitortsergent heitortsergent commented Jun 9, 2022 •
edited

Fixes #2178, #2148.

Checklist

  • Tests added / updated
  • Docs added / updated

Does this PR introduce a breaking change?

  • Yes
  • No

Additional context

I tried to make this document a little bit more beginner-friendly as I tried to run some of these examples myself while learning Spectral:

  • Replaced all require statements to import
  • Moved the mentions of CommonJS to its own section
  • Added a section explaining how to load multiple rulesets using the bundleAndLoadRuleset method
  • Added an example showing how to load an external specification file
  • Added link to JS ruleset section in Custom Rulesets page

Not sure if I tried to do too many changes here, would really appreciate some feedback!

@heitortsergent heitortsergent requested a review from P0lip June 9, 2022 22:00
@heitortsergent heitortsergent requested a review from a team as a code owner June 9, 2022 22:00
@heitortsergent heitortsergent changed the title docs(repo): Update Spectral in JavaScript examples docs(repo): update spectral in javascript examples Jun 10, 2022
@P0lip
Copy link
Contributor

P0lip commented Jun 10, 2022

@heitortsergent you could get https://app.circleci.com/pipelines/github/stoplightio/spectral/8590/workflows/3b058b3a-1fbf-4403-b316-6227242bf319/jobs/30750 step to pass?
How did you commit these changes? It's best to do that directly from the cloned project and avoid using tools that do not support git hooks.

P0lip
P0lip reviewed Jun 10, 2022
View reviewed changes
docs/guides/3-javascript.md Outdated
const { Spectral } = require("@stoplight/spectral-cli");
const { Resolver } = require("@stoplight/json-ref-resolver");
```js title="example-5.mjs" lineNumbers
import { join } from "path";
Copy link
Contributor

Choose a reason for hiding this comment

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

this probably won't work, because we use require to load the module.
Suggest to leave CJS here for now until we support ESM for custom resolvers

P0lip
P0lip reviewed Jun 10, 2022
View reviewed changes
docs/guides/3-javascript.md Outdated
## Linting a YAML String
### CommonJS and ESM

The examples on this page are written in ESM. If you're using CommonJS, you have to import an additional module:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only for custom functions. I don't think we cover custom functions on this page, thus I think we can remove it

@heitortsergent
Copy link
Contributor Author

@P0lip thanks for the comments!

I used Stoplight to make those changes. The CircleCI lint error there was related to the PR title, so I updated that. Now I'm getting a different error about the commit message...

⧗   input: Modified docs/guides/3-javascript.md
✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]
✖   scope may not be empty [scope-empty]

Not sure how to fix it... Should I rebase the commits and change the message so they follow the format docs(repo): {commit_message}?

@heitortsergent
Copy link
Contributor Author

I'm sorry, I just read the contributing guidelines 🤦 . Still wanna be sure this is the best way to fix the lint issues, should I rebase and fix the commit messages, and force push here?

@P0lip
Copy link
Contributor

P0lip commented Jun 13, 2022

Should I rebase the commits and change the message so they follow the format docs(repo): {commit_message}?

I can do that for you

@heitortsergent
Copy link
Contributor Author

@P0lip thanks for doing the rebase! I was having some issues with prettier but I think I got it all sorted out now, all checks are ✅ . :)

heitor.sergent@stoplight.io and others added 12 commits June 27, 2022 17:21
@heitortsergent
Copy link
Contributor Author

@P0lip are there any other changes you would like me to make before we can get this merged? :shipit:

@P0lip P0lip merged commit a782255 into develop Jun 28, 2022
@P0lip P0lip deleted the docs/spectral-js-updates branch June 28, 2022 19:10
@P0lip
Copy link
Contributor

P0lip commented Jun 28, 2022

Looks good!

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

@P0lip P0lip P0lip approved these changes

@philsturgeon philsturgeon Awaiting requested review from philsturgeon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update examples in Spectral in JavaScript docs
3 participants
@heitortsergent @P0lip @philsturgeon