Skip to content

Converting anyhow to this-error for re_renderer Renderer::draw #3487

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 7 commits into from
Sep 29, 2023

Conversation

chinepun
Copy link
Contributor

@chinepun chinepun commented Sep 26, 2023

What

  • Improves Use of thiserror in crates(#1845)

Checklist

@Wumpf Wumpf self-requested a review September 27, 2023 07:43
@Wumpf Wumpf changed the title Converting anyhow to this-error in High-level Crates Converting anyhow to this-error for re_renderer Renderer::draw Sep 27, 2023
@Wumpf Wumpf added enhancement New feature or request 🔺 re_renderer rendering, graphics, GPU include in changelog exclude from changelog PRs with this won't show up in CHANGELOG.md and removed include in changelog labels Sep 27, 2023
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!

Can you check if we can replace the anyhow based QueueableDrawDataError::DrawError variant already now? If not we can punt on that, but it would create a weird name-clash (unclear what's the difference between QueueableDrawDataError::DrawError and QueueableDrawDataError::DrawRenderer). But I think this can be replaced now (see comment)

CI is having issues right now with PRs from forks, looking into getting some coverage

@Wumpf
Copy link
Member

Wumpf commented Sep 27, 2023

CI is sadly a lost cause at the moment, only PRs from inside the repo work correctly right now. I'll run some tests locally later. See #1994. We'll look into that soon'ish

Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

nice, thanks!

@chinepun chinepun marked this pull request as ready for review September 29, 2023 09:48
@Wumpf
Copy link
Member

Wumpf commented Sep 29, 2023

counter checked locally for any compilation issues etc. since our ci still doesn't work well with forked repos (we'll look into this soonish)

@Wumpf Wumpf merged commit 3c953ca into rerun-io:main Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exclude from changelog PRs with this won't show up in CHANGELOG.md 🔺 re_renderer rendering, graphics, GPU
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants