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

feat(rome_cli): termination as diagnostic #4058

Merged
merged 10 commits into from
Dec 21, 2022
Merged

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Dec 15, 2022

Summary

This PR makes Termination a Diagnostic so that errors emitted from the CLI can be printed nicely using our diagnostic infrastructure.

To achieve that, the following changes have been made:

  • Termination is now called CliDiagnostic, it's an enum and it manually implements Diagnostic.
  • Each variant of the enum contains a diagnostic that is constructed using the diagnostic macro. That saved lot of work.
  • The different variants in CliDiagnostic are now implemented via functions.
  • RomeError has been renamed to WorkspaceError
  • Making CliDiagnostic a Diagnostic comes with a constraint where, to print it, we need a Console type. Now CliSession::new accepts a mutable reference to Console. The Console object gets created at the beginning of rome_cli/src/main.rs#main and then passed to CliSession. This change is also reflected in the test suite, unfortunately.
  • Error now implements the std trait Termination and returns ExitCode::FAILURE if the severity is >= Severity::Error.
  • the main() function now returns ExitCode
  • thiserror crate is removed
  • I updated some of the errors while I was at it; I suggest checking the new snapshots

Test Plan

I manually checked the snapshots and checked that they were correct.
I also ran some error cases locally and ensured that the CLI worked as expected.

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Dec 15, 2022

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 53e4148
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63a2d407c9d69a0008e05a4d
😎 Deploy Preview https://deploy-preview-4058--docs-rometools.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@ematipico ematipico force-pushed the feat/termination-diagnostic branch from b8529d8 to 0fbdd42 Compare December 16, 2022 17:52
@github-actions
Copy link

github-actions bot commented Dec 16, 2022

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 48647 48647 0
Passed 47582 47582 0
Failed 1065 1065 0
Panics 0 0 0
Coverage 97.81% 97.81% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6093 6093 0
Passed 1754 1754 0
Failed 4339 4339 0
Panics 0 0 0
Coverage 28.79% 28.79% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 565 565 0
Failed 74 74 0
Panics 0 0 0
Coverage 88.42% 88.42% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16740 16740 0
Passed 12817 12817 0
Failed 3923 3923 0
Panics 0 0 0
Coverage 76.57% 76.57% 0.00%

@ematipico ematipico marked this pull request as ready for review December 19, 2022 14:28
@ematipico ematipico requested review from leops, xunilrj, MichaReiser and a team as code owners December 19, 2022 14:28
@calibre-analytics
Copy link

calibre-analytics bot commented Dec 19, 2022

Your plan has been cancelled

Choose a plan to resume monitoring your Sites and Pull Requests. If you need help, check the Manage Your Plan and Test Usage guide.

@ematipico ematipico requested a review from leops December 20, 2022 11:20
@ematipico ematipico force-pushed the feat/termination-diagnostic branch from 4542c1f to 9acd081 Compare December 20, 2022 11:24
@ematipico ematipico added the A-CLI Area: CLI label Dec 20, 2022
@ematipico ematipico added this to the Next milestone Dec 20, 2022
@MichaReiser MichaReiser removed their request for review December 20, 2022 13:01
@ematipico ematipico force-pushed the feat/termination-diagnostic branch from cd5f559 to 53e4148 Compare December 21, 2022 09:38
@ematipico ematipico merged commit 0a70d1f into main Dec 21, 2022
@ematipico ematipico deleted the feat/termination-diagnostic branch December 21, 2022 10:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-CLI Area: CLI
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载