Skip to content

Review error messages #1683

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

Merged
merged 4 commits into from
May 24, 2024
Merged

Review error messages #1683

merged 4 commits into from
May 24, 2024

Conversation

olivroy
Copy link
Collaborator

@olivroy olivroy commented May 23, 2024

Summary

  • use standalone types check
  • Use error chaining
  • Use tidyverse style for errors.

For review, I'd suggest to look at code here, but if you want to have a visual feel of how this translates in real-life, I added links to important commits in #1681 that may help pinpoint the essence of the PR.

To have an idea of how this PR improves things.

Look at 472510f in #1681 which displays how certain messages improved.

30938f0 also helps to see the visual impact of some of my previous changes.

See #1681

Related GitHub Issues and PRs

Fixes #1681.
Related to #1638 and #1640.

I may add snapshot tests eventually, but given the size of the PR, I don't want to commit too many changes that may slow down development. (It took me a while to get all checks passing because of the size of the PR)

Checklist

@olivroy olivroy mentioned this pull request May 23, 2024
3 tasks
@rich-iannone rich-iannone self-requested a review May 24, 2024 17:13
Copy link
Member

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for this!

@rich-iannone
Copy link
Member

Let me know if you plan any more changes for this (just want to be sure of it).

@olivroy
Copy link
Collaborator Author

olivroy commented May 24, 2024

I was done with my changes here, unless changes are requested.

@rich-iannone rich-iannone merged commit 1c56395 into rstudio:master May 24, 2024
12 checks passed
@olivroy olivroy deleted the error-msg branch June 5, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants