search

stoplightio / spectral

A flexible JSON/YAML linter for creating automated style guides, with baked in support for OpenAPI (v3.1, v3.0, and v2.0), Arazzo v1.0, as well as AsyncAPI v2.x.
https://stoplight.io/spectral
Apache License 2.0
2.5k stars 238 forks source link

feat(core): add documentUrl to JS api and cli formatters #2443

Open dweber019 opened 1 year ago

dweber019 commented 1 year ago

Fixes https://github.com/stoplightio/spectral/issues/1488.

Checklist

Does this PR introduce a breaking change?

Screenshots

CleanShot 2023-03-29 at 23 51 42@2x CleanShot 2023-03-29 at 23 52 26@2x

All formatters have been updated.

dweber019 commented 1 year ago

Should we add the docuemtnUrl type here https://github.com/stoplightio/types/pull/127?

nahteb commented 1 year ago

thanks for raising this PR, we've just recently added documentationUrls to our ruleset and would find this really useful 👍

dweber019 commented 1 year ago

@magicmatatjahu @smoya @jonaslagoni any update on this?

jonaslagoni commented 1 year ago

I dont get why we are assigned tbh. @P0lip this is a core change, while it does change something for the AsyncAPI rules, this is all you 😄

mnaumanali94 commented 1 year ago

👋🏼 @dweber019 First of all, I would like to apologize for the delay in responding to your pull request. We appreciate your valuable contribution to the project.

We understand your intention to output the documentation URL as part of the lint process. However it may clutter the output and become repetitive. Additionally, not all organizations utilize the documentation URL, and some prefer using the description field.

With that in mind, we would like to propose a couple of alternative options:

Add the functionality to output the documentationURL behind the --verbose flag. This would make it an opt-in feature, giving users the choice to include the documentation URL in their lint output. The downside to this approach is that it may not cater to organizations that use the description field instead of the documentationURL. Introduce a new spectral ruleset explain command that displays information for each rule, including severity, documentationURL, and description. This would provide a more comprehensive overview of the rules without cluttering the lint output. We would love to hear your thoughts on these approaches or any other ideas you might have. Once we have your input, we can work together to make the necessary changes and integrate them into the project.

Thank you once again for your contribution, and we look forward to collaborating with you on this feature.

dweber019 commented 1 year ago

Hi @mnaumanali94

Thx for the feedback. I agree with you. It depends on the use case. How would you imagine this explain command.

  1. Is this a options flag on lint, and if a rule fails more output is generated
  2. or is it a standalone command to basically extract rule information from the rules file without linting?

For our use case, the first one would satisfy our needs and makes IMHO more sense, as the documentation url or description is needed in case of error mostly on CI side, where you don't like to rerun a command to understand the error.

shelleeboo commented 1 year ago

@dweber019 Thanks you for raising this issue and for your contribution. I have a similar requirement to make use of the documenationUrl property in Javascript.

@mnaumanali94 Any idea if this PR will be accepted and merged anytime soon?

Cheers!

mnaumanali94 commented 5 months ago

Is this a options flag on lint, and if a rule fails more output is generated

I would lean towards this

johannesmarx commented 5 months ago

@dweber019 thanks for taking care of this feature as I was also about to raise a similare request. Is there any reason, why no all formatters are considered?

IMHO also github-actions would heavily benefit from it as users simply can follow the docmentation links in GitHub pull requests.

Thanks

dweber019 commented 5 months ago

@dweber019 thanks for taking care of this feature as I was also about to raise a similare request. Is there any reason, why no all formatters are considered?

IMHO also github-actions would heavily benefit from it as users simply can follow the docmentation links in GitHub pull requests.

Thanks

No reason, github actions was not implemented when I last committed.

I'm just started to update this pr but have some harness binary issues.

dweber019 commented 5 months ago

harness could be solved by using node 18. I updated now everything and change the whole feature to a CLI flag. Additionally, github-actions and sarif is included now.

@mnaumanali94 could you do a review?

mnaumanali94 commented 3 months ago

LGTM!

dweber019 commented 3 months ago

@jonaslagoni @smoya could someone of you check this pr. Thanks a lot