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

[core] Fixed the problem of getting option values from groups #2891

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

ethouris
Copy link
Collaborator

Fixes #2887

Fixed: the group's getOpt returns one of the possible way:

  1. A groupwise option returns the value from its own fields
  2. A socketwise option is returned either from the option storage, or default values.
  3. Some options can't be requested on a group and for them error should be reported.

There was also added some options that should have been returend as groupwise, but were not updated accordingly.

@ethouris ethouris added Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core [tests] Area: Unit tests labels Feb 22, 2024
@ethouris
Copy link
Collaborator Author

Please ignore multiple commits. I have mistakelny taken a branch of another PR to check in the changes, only later merged to the right branch.

@maxsharabayko maxsharabayko added this to the v1.5.4 milestone Feb 22, 2024
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

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

Comparison is base (16cb043) 64.79% compared to head (2939a2b) 64.88%.

Files Patch % Lines
srtcore/group.cpp 35.71% 18 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2891      +/-   ##
==========================================
+ Coverage   64.79%   64.88%   +0.08%     
==========================================
  Files         101      101              
  Lines       17482    17540      +58     
==========================================
+ Hits        11328    11381      +53     
- Misses       6154     6159       +5     

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

@maxsharabayko
Copy link
Collaborator

Would it be possible to rebase to exclude the merge commit? Otherwise I'd have to squash merge.

@ethouris
Copy link
Collaborator Author

Ok, I'll recreate the branch

@maxsharabayko
Copy link
Collaborator

maxsharabayko commented Feb 23, 2024

So you squash-merged it yourself... I intended to preserve those two commits: [core] and [tests]. That was the reason for my question about excluding the merge commit. I could then rebase those two commits on the master branch.

@ethouris
Copy link
Collaborator Author

I didn't squash-merge. Both the fix and the test were submitted in a single commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core [tests] Area: Unit tests Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Group socket without members always return default value of options
2 participants