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: Give enum-from-int functions the ability to report errors. #2475

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

iphydf
Copy link
Member

@iphydf iphydf commented Dec 18, 2023

These functions are a bit clearer and don't need to change if enum values change.

See TokTok/hs-tokstyle#212 for the relevant linter implementation.


This change is Reviewable

@iphydf iphydf added this to the v0.2.19 milestone Dec 18, 2023
@iphydf iphydf marked this pull request as ready for review December 18, 2023 14:00
Copy link

codecov bot commented Dec 18, 2023

Codecov Report

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

Comparison is base (19d8f18) 69.90% compared to head (4e603bb) 69.90%.

Files Patch % Lines
toxcore/tox_unpack.c 14.49% 118 Missing ⚠️
toxcore/Messenger.c 40.00% 9 Missing ⚠️
toxcore/group_pack.c 77.77% 4 Missing ⚠️
toxcore/events/conference_invite.c 0.00% 1 Missing ⚠️
toxcore/events/conference_message.c 0.00% 1 Missing ⚠️
toxcore/events/file_recv_control.c 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2475      +/-   ##
==========================================
- Coverage   69.90%   69.90%   -0.01%     
==========================================
  Files          75       75              
  Lines       25964    25999      +35     
==========================================
+ Hits        18150    18174      +24     
- Misses       7814     7825      +11     

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

@iphydf iphydf force-pushed the enum-from-int branch 11 times, most recently from 0f545bd to a8e35fe Compare December 19, 2023 19:52
These functions are a bit clearer and don't need to change if enum
values change.

See TokTok/hs-tokstyle#212 for the relevant
linter implementation.
Copy link
Member

@JFreegman JFreegman left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 approvals obtained (waiting on @iphydf)


toxcore/group_chats.c line 1283 at r3 (raw file):

    len_processed += sizeof(uint8_t);

    group_voice_state_from_int(voice_state, &shared_state->voice_state);

since these functions return bool do we want to log failures?


toxcore/Messenger.c line 785 at r3 (raw file):

        default: {
            *out = USERSTATUS_INVALID;

I'm not 100% sure this should be the default. Maybe just setting it to NONE would prevent cascading failures?

Copy link
Member

@JFreegman JFreegman left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 7 files at r2, 12 of 12 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @iphydf)

@iphydf iphydf changed the title refactor: Use enum-from-int rule from tokstyle. refactor: Give enum-from-int functions the ability to report errors. Dec 19, 2023
@iphydf iphydf merged commit 4e603bb into TokTok:master Dec 19, 2023
50 of 51 checks passed
@iphydf iphydf deleted the enum-from-int branch December 19, 2023 21:25
This pull request was closed.
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