Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

docs(rome_diagnostics): add a contributor documentation file for diagnostics #3401

Merged
merged 3 commits into from
Oct 19, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Oct 11, 2022

Summary

This PR adds a new file in crates/rome_diagnostics/CONTRIBUTING.md that serves as a documentation on how to write diagnostics, with both technical explanations and best practices to write good errors for the user.

Test Plan

N/A

@leops leops temporarily deployed to netlify-playground October 11, 2022 14:13 Inactive
@netlify
Copy link

netlify bot commented Oct 11, 2022

Deploy Preview for rometools canceled.

Name Link
🔨 Latest commit 1a474cb
🔍 Latest deploy log https://app.netlify.com/sites/rometools/deploys/6348094e9f364400087178a1

@github-actions
Copy link

github-actions bot commented Oct 11, 2022

crates/rome_diagnostics/CONTRIBUTING.md Outdated Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Outdated Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Outdated Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Outdated Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Outdated Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Outdated Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Outdated Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Outdated Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Show resolved Hide resolved
crates/rome_diagnostics/CONTRIBUTING.md Show resolved Hide resolved
@leops leops temporarily deployed to netlify-playground October 13, 2022 09:18 Inactive
@leops leops temporarily deployed to netlify-playground October 13, 2022 12:49 Inactive
@ematipico
Copy link
Contributor

ematipico commented Oct 14, 2022

Here's some feedback after working with the new diagnostics:

  • it wasn't clear that diagnostics can have their source code and file injected via some external API (.with_file_path and with_file_source_code), if these diagnostics need to show some code frame
  • if the diagnostic need to show an error as code frame, they need to a span assigned to them
  • it would be nice to have an utility to print diagnostics in tests (we had an Emitter, we could have it for these new diagnostics too)
  • we should explain when a diagnostic can be converted to Error. I managed to pick it up, but we should explain some use case

@ematipico
Copy link
Contributor

@leops should we start merging this PR and then continue with other PRs? There haven't been updates recently

@leops
Copy link
Contributor Author

leops commented Oct 19, 2022

@leops should we start merging this PR and then continue with other PRs? There haven't been updates recently

Ah yes I wanted to add an additional section that show what a diagnostic looks like when it gets printed along with all the advice types, but we could merge this now and extend it later

@leops leops marked this pull request as ready for review October 19, 2022 12:27
@leops leops requested a review from a team October 19, 2022 12:27
@ematipico ematipico merged commit 745464f into main Oct 19, 2022
@ematipico ematipico deleted the docs/diagnostics-contributor branch October 19, 2022 13:30
@leops leops added A-Diagnostic Area: errors and diagnostics A-Documentation Area: documentation labels Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Diagnostic Area: errors and diagnostics A-Documentation Area: documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants