build: Bump rust-minidump #681
Conversation
| @@ -1512,10 +1515,7 @@ impl SymbolProvider for TempSymbolProvider { | |||
| _frame: &mut dyn minidump_processor::FrameSymbolizer, | |||
| ) -> Result<(), minidump_processor::FillSymbolError> { | |||
| // TODO(ja): Deduplicate this. Probably should use a different map key, ... | |||
There was a problem hiding this comment.
I have a sneaking suspicion that maybe we can delete this now, but maybe it'd be good to confirm this with @jan-auer to be safe.
| @@ -1534,7 +1534,7 @@ impl SymbolProvider for TempSymbolProvider { | |||
| walker: &mut dyn minidump_processor::FrameWalker, | |||
| ) -> Option<()> { | |||
| // TODO(ja): Deduplicate this. Probably should use a different map key, ... | |||
|
Took a peek at the CI failures and I left a comment on a possible cause of the failure. As Floris mentioned above, it looks like a few comments could be removed as a result of this change, so I've left some notes pointing those out. |
Codecov Report
@@ Coverage Diff @@
## release/rust-minidump #681 +/- ##
=========================================================
- Coverage 71.49% 71.47% -0.03%
=========================================================
Files 47 47
Lines 11019 11018 -1
=========================================================
- Hits 7878 7875 -3
- Misses 3141 3143 +2
Continue to review full report at Codecov.
|
| } | ||
|
|
||
| RawObjectInfo { | ||
| ty, | ||
| // TODO: shouldn't this be None if code_id is empty? |
There was a problem hiding this comment.
calling this TODO out since I added this in, maybe I just missed something? maybe Some("") is meaningful here?
|
instead of writing a wall of text i went and just fixed the CI. turns out you don't need to do any of the additional processing of the debug id inherited from breakpad because debug ids from sorry if this feels like i stepped on your toes a little bit, just figured i'd just fix this up. i was really hoping i could actually update the |
relaxolotl
force-pushed
the
rust-minidump/debugid
branch
from
5a4fda2 to
5e60125
Compare
| // TODO(ja): This is optional now, wasn't before, check why | ||
| debug_id: module.debug_identifier().map(|c| c.into_owned()), | ||
| debug_id: module.debug_identifier().map(|id| id.to_string()), |
There was a problem hiding this comment.
i think this is wrong because previously we were handling strings from breakpad while this does not use the breakpad formatting of DebugId.
Our rust-minidump branch now contains this commit, cherry-picked from upstream. Consequently we need to update the dependency in this branch to make everything work with
DebugId.#skip-changelog