Skip to content

chore: tox_new() should return null when savedata loading fails#1944

Merged
JFreegman merged 1 commit into
TokTok:masterfrom
JFreegman:fix_docs
Nov 15, 2023
Merged

chore: tox_new() should return null when savedata loading fails#1944
JFreegman merged 1 commit into
TokTok:masterfrom
JFreegman:fix_docs

Conversation

@JFreegman
Copy link
Copy Markdown
Member

@JFreegman JFreegman commented Jan 31, 2022 •
edited by iphydf
Loading

Returning a valid tox instance when loading a corrupt savefile is probably not desired behaviour

This change is Reviewable

@JFreegman JFreegman added api break Change breaks API or ABI chore Updating grunt tasks etc; no production code change labels Jan 31, 2022
@JFreegman JFreegman added this to the v0.2.x milestone Jan 31, 2022
@auto-add-label auto-add-label Bot added the api label Jan 31, 2022
@JFreegman JFreegman changed the title api: tox_new() should return null when savedata loading fails chore: tox_new() should return null when savedata loading fails Jan 31, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 31, 2022 •
edited
Loading

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (32041a1) 74.41% compared to head (8792672) 74.51%.
Report is 1 commits behind head on master.

❗ Current head 8792672 differs from pull request most recent head fc623a5. Consider uploading reports for the commit fc623a5 to get more accurate results

Files Patch % Lines
toxcore/tox.c 20.00% 8 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1944      +/-   ##
==========================================
+ Coverage   74.41%   74.51%   +0.10%     
==========================================
  Files          87       87              
  Lines       26235    26241       +6     
==========================================
+ Hits        19523    19554      +31     
+ Misses       6712     6687      -25

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

@settings settings Bot removed the api label Feb 2, 2022
@JFreegman JFreegman force-pushed the fix_docs branch 3 times, most recently from cfca43d to 2b89506 Compare February 5, 2022 14:33
@iphydf iphydf added P3 Low priority and removed P3 Low priority labels Feb 5, 2022
@iphydf iphydf modified the milestones: v0.2.x, v0.3.0, v0.2.15 Feb 5, 2022
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 5, 2022 •
edited
Loading

✔️ Deploy Preview for c-toxcore ready!

🔨 Explore the source changes: 58806ff9bc1baee43eb82b7cc23dcf69ad1233c4

🔍 Inspect the deploy log: https://app.netlify.com/sites/c-toxcore/deploys/620dbd3d5a7e34000a1e82b7

😎 Browse the preview: https://deploy-preview-1944--c-toxcore.netlify.app

@robinlinden robinlinden modified the milestones: v0.2.15, v0.2.16 Feb 6, 2022
@iphydf iphydf modified the milestones: v0.2.16, v0.3.0 Feb 6, 2022
@JFreegman JFreegman force-pushed the fix_docs branch 2 times, most recently from 8f7c3a2 to a36e24f Compare February 10, 2022 16:43
@JFreegman JFreegman force-pushed the fix_docs branch 2 times, most recently from 10e1aed to 58806ff Compare February 17, 2022 03:12
@JFreegman JFreegman force-pushed the fix_docs branch 2 times, most recently from 57e4dc8 to e6af91b Compare February 23, 2022 21:10
@iphydf
Copy link
Copy Markdown
Member

iphydf commented Mar 13, 2022

Maybe it is. It probably makes sense to be a bit lenient with slightly broken savedatas.

@iphydf
Copy link
Copy Markdown
Member

iphydf commented Nov 12, 2023

Ok, I've changed my mind. However, I think that the loading code should be quite accepting, and return CONTINUE more often, and only return ERROR if the error is not recoverable. That can be done later, though.

@Green-Sky Green-Sky mentioned this pull request Nov 15, 2023
Open
@JFreegman JFreegman force-pushed the fix_docs branch 5 times, most recently from 5efeb8a to 17282e7 Compare November 15, 2023 03:01
@JFreegman
tox_new() should return null when savedata loading fails
fc623a5 
Returning a valid tox instance when loading a corrupt
savefile is probably not desired behaviour
@iphydf iphydf modified the milestones: v0.3.0, v0.2.19 Nov 15, 2023
iphydf
iphydf reviewed Nov 15, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@iphydf iphydf left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@JFreegman JFreegman merged commit fc623a5 into TokTok:master Nov 15, 2023
@JFreegman JFreegman deleted the fix_docs branch November 15, 2023 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iphydf iphydf iphydf approved these changes

Assignees

@JFreegman JFreegman

Labels

api break Change breaks API or ABI chore Updating grunt tasks etc; no production code change

Milestone

v0.2.19

Development

Successfully merging this pull request may close these issues.

3 participants

@JFreegman @iphydf @robinlinden