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

Refactor usage of unwrap in re_db, re_format_arrow & re_log_types #6380

Merged
merged 7 commits into from
May 27, 2024

Conversation

Artxiom
Copy link
Contributor

@Artxiom Artxiom commented May 17, 2024

What

Part of #6330
Removes or explicitly allows unwrap() where it makes sense.

Affected crates: re_entity_db, re_format_arrow, and re_log_types

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@Artxiom Artxiom force-pushed the 6330-refactor-unwrap-re_log_types+ branch from 1258b8e to 5337b72 Compare May 17, 2024 19:12
@Artxiom Artxiom marked this pull request as ready for review May 17, 2024 19:14
@emilk emilk self-assigned this May 20, 2024
crates/re_log_types/src/data_table.rs Outdated Show resolved Hide resolved
crates/re_log_types/src/data_cell.rs Show resolved Hide resolved
crates/re_log_types/src/data_cell.rs Show resolved Hide resolved
crates/re_log_types/src/data_cell.rs Show resolved Hide resolved
crates/re_log_types/src/data_cell.rs Show resolved Hide resolved
crates/re_log_types/src/time.rs Outdated Show resolved Hide resolved
@Artxiom Artxiom force-pushed the 6330-refactor-unwrap-re_log_types+ branch from 6974279 to 7df8a8f Compare May 21, 2024 18:40
@Artxiom Artxiom requested a review from emilk May 21, 2024 18:44
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Thanks!

@emilk emilk added 🧑‍💻 dev experience developer experience (excluding CI) exclude from changelog PRs with this won't show up in CHANGELOG.md labels May 21, 2024
@emilk
Copy link
Member

emilk commented May 21, 2024

there are some CI failures

@Artxiom
Copy link
Contributor Author

Artxiom commented May 23, 2024

there are some CI failures

I had something similar in my previous PR #6337. Can you check in with @Wumpf? he mentioned that this is probably some issue withe the contributor ci.

@Wumpf
Copy link
Member

Wumpf commented May 23, 2024

@Artxiom the one on should be fixed by now but that's also not what was hit here

@Artxiom
Copy link
Contributor Author

Artxiom commented May 23, 2024

@Artxiom the one on should be fixed by now but that's also not what was hit here

Aaargh...you are right, I'm sorry - I overlooked smthg. Will fix it now.

@Wumpf Wumpf merged commit 36ad454 into rerun-io:main May 27, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI) exclude from changelog PRs with this won't show up in CHANGELOG.md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants