Skip to content

Comments

Simplify Deserializer::peek to avoid unreachable#860

Merged
Mingun merged 1 commit intotafia:masterfrom
Dr-Emann:push-oqkoknnmvqxm
May 5, 2025
Merged

Simplify Deserializer::peek to avoid unreachable#860
Mingun merged 1 commit intotafia:masterfrom
Dr-Emann:push-oqkoknnmvqxm

Conversation

@Dr-Emann
Copy link
Contributor

@Dr-Emann Dr-Emann commented May 4, 2025

Use Option::insert which allows getting a reference to the value just inserted into the Option.

Unfortunately, we can't simplify this to just Option::get_or_insert_with because we can fail to generate the value to insert (if reader.next() fails).

@codecov-commenter
Copy link

codecov-commenter commented May 4, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.74%. Comparing base (a9391f3) to head (254fbd2).
Report is 58 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #860      +/-   ##
==========================================
+ Coverage   60.21%   60.74%   +0.53%     
==========================================
  Files          41       41              
  Lines       16021    16044      +23     
==========================================
+ Hits         9647     9746      +99     
+ Misses       6374     6298      -76     
Flag Coverage Δ
unittests 60.74% <100.00%> (+0.53%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@Mingun Mingun left a comment

Choose a reason for hiding this comment

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

Good catch. Just add missing comma and I merge it.

Use `Option::insert` which allows getting a reference to the value just
inserted into the Option.

Unfortunately, we can't simplify this to just
`Option::get_or_insert_with` because we can fail to generate the value
to insert (if reader.next() fails).
@Dr-Emann Dr-Emann force-pushed the push-oqkoknnmvqxm branch from be41ced to 254fbd2 Compare May 4, 2025 14:52
@Mingun Mingun merged commit 2369f01 into tafia:master May 5, 2025
7 checks passed
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.

3 participants