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.

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

Move getRuleDescriptionUrl to plugins #151

Merged
merged 8 commits into from
Dec 24, 2024

Conversation

daniilsapa
Copy link
Collaborator

Closes #131

I was not sure about one thing: should getRuleDescriptionUrl be an optional plugin method or not? I decided to make it required because otherwise, a diagnostic is not that useful. What do you think?

@daniilsapa daniilsapa requested a review from illright December 19, 2024 14:08
Copy link

changeset-bot bot commented Dec 19, 2024

🦋 Changeset detected

Latest commit: ff9ffe9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@feature-sliced/steiger-plugin Minor
steiger Minor
@steiger/types Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

packages/steiger/src/app.ts Outdated Show resolved Hide resolved
packages/steiger/src/models/config/index.ts Outdated Show resolved Hide resolved
packages/steiger/src/models/config/validate-config.spec.ts Outdated Show resolved Hide resolved
packages/steiger/src/models/config/index.ts Show resolved Hide resolved
@illright
Copy link
Member

About optionality — I think I would prefer it to be optional to lower the entry barrier for writing rules. It feels strange to link to documentation that doesn't yet exist, so we want to let people add the docs when it's convenient. If they don't have docs, they can always make the error message more descriptive

@daniilsapa
Copy link
Collaborator Author

Actually, optionality makes sense in this case. I think I'll change it a bit so that function can provide either a link or a short description for a diagnostic. Is that something you thought too?

@illright
Copy link
Member

I'll change it a bit so that function can provide either a link or a short description for a diagnostic. Is that something you thought too?

No, I meant that the explanation can be given in the error message itself. How would you like to display this short description that a function might return?

@daniilsapa
Copy link
Collaborator Author

I was thinking about a short description that could be returned by the getRuleDescription function. Then we could display that instead of a link. But I looked at our diagnostics again and saw that a short description instead of a link is a bit of nonsense. So never mind...

@daniilsapa
Copy link
Collaborator Author

I made getRuleDescriptionUrl optional

Copy link
Member

@illright illright left a comment

Choose a reason for hiding this comment

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

All good!

@illright illright merged commit a1b458c into next Dec 24, 2024
7 checks passed
@illright illright deleted the feature/move-get-rule-description branch December 24, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants