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.

Sign up for GitHub

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

Jump to bottom

Make ProtectionProfile methods public #260

Merged
merged 1 commit into from
Jul 8, 2024
Merged

Make ProtectionProfile methods public #260

merged 1 commit into from
Jul 8, 2024

Conversation

sirzooro
Copy link
Contributor

Description

Make ProtectionProfile methods public. This allows to get values like key length and use them with key management protocols like MIKEY, instead of creating own constants for this.

Reference issue

Fixes #258

@codecov Codecov
Copy link

codecov bot commented Feb 24, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.32%. Comparing base (e9fc319) to head (0c244b8).

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #260      +/-   ##
==========================================
- Coverage   78.83%   78.32%   -0.51%     
==========================================
  Files          17       17              
  Lines         992      992              
==========================================
- Hits          782      777       -5     
- Misses        118      124       +6     
+ Partials       92       91       -1
Flag Coverage Δ
go ?
wasm 78.32% <100.00%> (ø)

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.

at-wat
at-wat reviewed Feb 24, 2024
View reviewed changes
Copy link
Member

@at-wat at-wat left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Added comment on naming.
The changes look good to me!

srtp_cipher.go Outdated
Comment on lines 13 to 17
RtpAuthTagLen() (int, error)
RtcpAuthTagLen() (int, error)
// aeadAuthTagLen returns AEAD auth key length of the cipher.
// See the note below.
aeadAuthTagLen() (int, error)
AeadAuthTagLen() (int, error)
Copy link
Member

Choose a reason for hiding this comment

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

Acronyms are all capitals in Go's naming convention. So RTPAuth*, RTCPAuth*, AEADAuth* would be preferred

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@sirzooro
Copy link
Contributor Author

I have changed names as requested and added comments to make linter happy.

Please take a look on test pipeline, it failed with following error:
[2024-02-24T04:11:55.282Z] ['error'] There was an error running the uploader: Error uploading to https://codecov.io: Error: There was an error fetching the storage URL during POST: 404 - {'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}

@Sean-Der
Copy link
Member

@sirzooro Great!

Can you squash those commits into one. Use your description on GitHub as the commit message.

I added you to the GitHub org so the tests should run (without our intervention). When it goes green we can merge :)

@sirzooro sirzooro force-pushed the make_protection_profile_methods_public branch 3 times, most recently from aadc5e4 to 3fff0dd Compare February 24, 2024 21:08
@sirzooro
Copy link
Contributor Author

Done :)

@Sean-Der Sean-Der force-pushed the make_protection_profile_methods_public branch from 3fff0dd to 4ce5dff Compare February 25, 2024 01:13
@Sean-Der
Copy link
Member

Perfect! Thank you so much @sirzooro

Feel free to push a tag if you need one. git tag v3.0.1 && git push --tags is all the process you need :)

@sirzooro sirzooro force-pushed the make_protection_profile_methods_public branch from 4ce5dff to 0308887 Compare July 7, 2024 15:40
@sirzooro sirzooro requested a review from at-wat July 7, 2024 15:42
at-wat
at-wat approved these changes Jul 8, 2024
View reviewed changes
Copy link
Member

@at-wat at-wat left a comment

Choose a reason for hiding this comment

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

LGTM

@sirzooro
Make ProtectionProfile methods public
0c244b8 
This allows to get values like key length and use them with key
management protocols like MIKEY, instead of creating own constants
for this.

Resolves pion#258
@sirzooro sirzooro force-pushed the make_protection_profile_methods_public branch from 0308887 to 0c244b8 Compare July 8, 2024 21:54
@sirzooro sirzooro merged commit e3e6d11 into pion:master Jul 8, 2024
14 checks passed
@sirzooro sirzooro deleted the make_protection_profile_methods_public branch July 8, 2024 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@at-wat at-wat at-wat approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Please make ProtectionProfile methods public
3 participants
@sirzooro @Sean-Der @at-wat