Skip to content

Conversation

@NickCondron
Copy link
Contributor

@NickCondron NickCondron commented Jan 21, 2023

I think this simplifies the code a little and reduces the likelihood of introducing a panic in the future. Maybe in the future we would want to log something if we see an unsupported section.

Btw, I'm curious why so much work is done to convert object::* types to various obj::Obj* types? Are there some features of the object crate that are incompatible with objdiff's goals?

@encounter
Copy link
Owner

The panic happens on a separate thread, where the job system will pick up the error and present it normally. I may prefer replacing the panic! with anyhow::bail!, so it returns an error gracefully. But this change looks good for now.

Converting to the Obj* types is for several reasons:

  • Mutability; the object types are thin wrappers over immutable data
  • Ease of accessing associated data, like the diff results
  • Not tying the code super heavily to the object crate, other loaders could be implemented in the future

@encounter encounter merged commit 6afc535 into encounter:main Jan 21, 2023
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