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

docs(website): include rule path in rule pages #321

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

Vivalldi
Copy link
Contributor

@Vivalldi Vivalldi commented Sep 18, 2023

Summary

Motivation: this helps know which group the rule belongs. Otherwise readers have to rely on the generated valid/invalid outputs to know the full rule name.

image

Open to feedback on how this should look

Test Plan

  • Look at generated lint rule pages

@github-actions github-actions bot added A-Tooling Area: internal tools A-Website Area: website labels Sep 18, 2023
@ematipico
Copy link
Member

ematipico commented Sep 18, 2023

This change is very appreciated, I was looking to do something similar.

I suggest a few changes, let me know what you think:

  • I would call it "diagnostic category". That's really what it is. Maybe it can be misleading because we use it in suppression comments too;
  • rules pages are all auto-generated from source code, you'll need to change xtasks/lintdoc/src/lib.rs

@Vivalldi
Copy link
Contributor Author

Vivalldi commented Sep 18, 2023

  • I would call it "diagnostic category". That's really what it is. Maybe it can be misleading because we use it in suppression comments too;

Love it! Any formatting suggestions? Things I've considered

  • Just using bold (current)
  • Headers (all seemed a bit too big)
  • A subtitle below main title (thought this would be ideal but found wasn't as easy to add to template as thought)
  • rules pages are all auto-generated from source code, you'll need to change xtasks/lintdoc/src/lib.rs

Would this be in addition to the changes made in xtask/lintdoc/src/main.rs? Or should I only make the change in lib.rs? I don't see a xtasks/lintdoc/src/lib.rs, only main.rs

@Vivalldi Vivalldi force-pushed the vivalldi/include-rule-path-in-docs branch from 481bb39 to 0b8ee1c Compare September 18, 2023 13:35
@ematipico
Copy link
Member

Yeah sorry, it's main.rs. Didn't remember 🥲

I like the current proposal!

@Vivalldi Vivalldi force-pushed the vivalldi/include-rule-path-in-docs branch from 0b8ee1c to 7b607ba Compare September 19, 2023 19:08
@ematipico ematipico merged commit 4a12cfd into biomejs:main Sep 20, 2023
14 checks passed
@Vivalldi Vivalldi deleted the vivalldi/include-rule-path-in-docs branch September 20, 2023 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Tooling Area: internal tools A-Website Area: website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants