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

feat: graphically display error when adb command fails #409

Merged
merged 5 commits into from
Apr 12, 2024

Conversation

lavafroth
Copy link
Contributor

@lavafroth lavafroth commented Apr 4, 2024

This is more or less scaffold code to display the raw error if an ADB command fails.
Related to #37

  • We use an enum to classify these errors. Currently it only has one error type as Generic.
  • Any ADB error will display a modal with the raw error trace of the ADB command itself.

Screenshot from 2024-04-05 19-25-56

@AnonymousWP AnonymousWP added enhancement New feature or request UI Related to the UI labels Apr 4, 2024
@AnonymousWP
Copy link
Member

Could you please include a screenshot for references? Then we can also give feedback about that.

@lavafroth
Copy link
Contributor Author

I've added the screenshot

@adhirajsinghchauhan adhirajsinghchauhan requested review from a team and removed request for adhirajsinghchauhan April 6, 2024 10:44
@adhirajsinghchauhan
Copy link
Contributor

I'm busy, so I'll let other devs review this

Copy link
Contributor

deepsource-io bot commented Apr 9, 2024

Here's the code health analysis summary for commits 75287ba..2948d5e. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Rust LogoRust❌ Failure
❗ 5 occurences introduced
🎯 5 occurences resolved
View Check ↗
DeepSource Test coverage LogoTest coverage⚠️ Artifact not reportedTimed out: Artifact was never reportedView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@lavafroth
Copy link
Contributor Author

I've added a scrollbar to the error text. Should be fine for a merge.

@AnonymousWP
Copy link
Member

This can't be merged though due to failing checks.

src/core/sync.rs Outdated Show resolved Hide resolved
@lavafroth
Copy link
Contributor Author

lavafroth commented Apr 12, 2024 via email

Copy link
Member

@Frigyes06 Frigyes06 left a comment

Choose a reason for hiding this comment

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

Nice work, please add more comments next time :)

@Frigyes06 Frigyes06 merged commit 9b82148 into main Apr 12, 2024
17 of 19 checks passed
@Frigyes06 Frigyes06 deleted the show-adb-error branch April 12, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request UI Related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants