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

Implement individual linter rule pages #4999

Merged
merged 11 commits into from
Jun 27, 2023
Merged

Conversation

parlough
Copy link
Member

@parlough parlough commented Jun 17, 2023

This PR is meant to finalize work to make dart.dev a satisfactory replacement for dart-lang.github.io/linter/ while dart-lang/linter moves to the SDK and we work on consolidating and improving documentation (#4498).

It does this by (staged links after colon (:)):

  • Updating the information on dart.dev/lints for the latest state of the linter: /lints
  • Making dart.dev/lints an index page rather than including all information: /lints
  • Moving the long-form lint rule documentation to individual pages found at dart.dev/lints/<rule>: /lints/avoid_dynamic_calls
    • It does this by using a Jekyll page generator based on linter_rules.json.
  • Adding a textual reference pointing to the recently released all linter rules page (38973de) found at: /lints/all
  • Updating and links and references for these previous changes (mostly Effective Dart).

Contributes to dart-lang/linter#4460, #4498, #4499

Note: This is an intermediate step and doesn't aim to improve the formatting or style of the information on the index or individual pages. That will be part of follow-up work as we work on improvements to the diagnostic linter rule documentation.

@github-actions
Copy link

github-actions bot commented Jun 17, 2023

Visit the preview URL for this PR (updated for commit f5ea40b):

https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app

(expires Tue, 04 Jul 2023 17:30:35 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: d851bc446d3c4d7394c5406c6f07255afc7075f3

@parlough parlough added review.copy Awaiting Copy Review review.tech Awaiting Technical Review labels Jun 17, 2023
@parlough parlough changed the title [WIP] Implement individual linter rule pages Implement individual linter rule pages Jun 18, 2023
@parlough parlough marked this pull request as ready for review June 18, 2023 19:04
Copy link
Member

@bwilkerson bwilkerson left a comment

Choose a reason for hiding this comment

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

I don't know the infrastructure being used well enough to catch problems, so if there are areas of concern you'll probably want a review from someone else.

@parlough parlough removed the request for review from pq June 18, 2023 22:10
@parlough parlough removed the review.tech Awaiting Technical Review label Jun 20, 2023
@MaryaBelanger
Copy link
Contributor

This is really cool. The new structure and navigation between pages feels fluid and looks great. Clutter is significantly down.

For my own understanding, are the individual lint pages actually created (like, under tools/linter-rules there's an actual .md file for each), or is each one just spun up dynamically?

I'm wondering about the URLs. I was expecting the actual URL to be dart.dev/lints/<etc> and the old tools/linter-rules/ URLs would redirect to that, but it seems like it's the other way around. I'm assuming it might be too much/too complicated to actually change the file structure around? Or maybe I'm off, could you explain it?

@parlough
Copy link
Member Author

@MaryaBelanger

For my own understanding, are the individual lint pages actually created (like, under tools/linter-rules there's an actual .md file for each), or is each one just spun up dynamically?

This was a bit tricky to get working properly and I plan to document it as I improve the functionality, especially since I hope to make it generic enough to support diagnostic messages too. The summary is that they are generated automatically during the Jekyll build (currently from the linter_rules.json), like they are Markdown files under tools/linter-rules (which you'll see in the built site), but you won't see them in your src directory.

For a longer explanation: The Jekyll plugin in src/_plugins/linter_page_generator.rb generates each Markdown page with the layout src/_layouts/linter-rule-standalone.html, which uses the src/_includes/linter-page-content.md include to generate the page's content. I had to have a separate layout and include file because I guess layout files have to HTML. I could have done it in the Ruby plugin, but the Markdown file seemed more friendly to edit and review, since not many of us (including me) know Ruby well.

I'm wondering about the URLs. I was expecting the actual URL to be dart.dev/lints/ and the old tools/linter-rules/ URLs would redirect to that, but it seems like it's the other way around.

I tried to keep it the same as before intentionally, as I feel it still fits within the other tools content (especially once/if we enable breadcrumbs), and the redirect is just a link destination we can guarantee will continue to work, no matter where the content actually lives. Let me know if you think this content should be directly under /lints though, it's an option, just perhaps with less flexibility for moving in the future.

@MaryaBelanger
Copy link
Contributor

Let me know if you think this content should be directly under /lints

Nope, your explanation makes sense so let's keep it the way you've set it up so far.

For a longer explanation:

Couple more questions regarding this paragraph (but feel free to merge in the meantime). I know this design/plan might not be fully figured out yet, but wondering if it is/what you're thinking:

  • For the "justification" content per lint, will that live on these generated pages? And if yes, how so? Will it still be written into the yaml files in the sdk?
  • It sounds like we won't be able to manually adjust these generated pages in this repo -- I'm not sure how important this might be to the plan so I just want to hear your thoughts on it

I'm still catching up on notes from our last meeting / what you worked on while I was out, sorry I keep asking you to re-explain everything! I'll be caught up soon, and will also sync with Brian if these questions are beyond your scope of the project 🤞

@parlough
Copy link
Member Author

parlough commented Jun 27, 2023

For the "justification" content per lint, will that live on these generated pages? And if yes, how so? Will it still be written into the yaml files in the sdk?

Yes that is what I'm thinking. I don't think we've fully decided where the source justification content will live or in what format, but it sounds like it may be better in the SDK so it can be used there as well. I'm not exactly sure when, but perhaps when you hover a linter rule or whatever. In the meantime, it is still coming from the machine-generated file on dart-lang/linter.

It sounds like we won't be able to manually adjust these generated pages in this repo -- I'm not sure how important this might be to the plan so I just want to hear your thoughts on it

Similar to the previous question, it's not clear where we will store the content eventually. But most likely, the format and layout will continue to be adjustable and controllable in this repo at the very least.

@parlough parlough merged commit 2faf0e4 into main Jun 27, 2023
@parlough parlough deleted the feature/linter-individual-pages branch June 27, 2023 17:33
rmacnak-google pushed a commit to rmacnak-google/site-www that referenced this pull request Sep 5, 2023
This PR is meant to finalize work to make dart.dev a satisfactory
replacement for
[`dart-lang.github.io/linter/`](https://dart-lang.github.io/linter/)
while `dart-lang/linter` moves to the SDK and we work on consolidating
and improving documentation
(dart-lang#4498).

It does this by (staged links after colon (`:`)):

- Updating the information on dart.dev/lints for the latest state of the
linter:
[/lints](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/tools/linter-rules)
- Making dart.dev/lints an index page rather than including all
information:
[/lints](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/tools/linter-rules)
- Moving the long-form lint rule documentation to individual pages found
at `dart.dev/lints/<rule>`:
[/lints/avoid_dynamic_calls](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/lints/avoid_dynamic_calls)
- It does this by using a Jekyll page generator based on
`linter_rules.json`.
- Adding a textual reference pointing to the recently released all
linter rules page
(dart-lang@38973de)
found at:
[/lints/all](https://dart-dev--pr4999-feature-linter-indiv-zetj76b0.web.app/lints/all)
- Updating and links and references for these previous changes (mostly
Effective Dart).

Contributes to dart-lang/linter#4460,
dart-lang#4498,
dart-lang#4499

**Note:** This is an intermediate step and doesn't aim to improve the
formatting or style of the information on the index or individual pages.
That will be part of follow-up work as we work on improvements to the
diagnostic linter rule documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review.copy Awaiting Copy Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants