Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
plugins/janus_sip.c: MESSAGE Authentication and Deliver Status Report #2786
plugins/janus_sip.c: MESSAGE Authentication and Deliver Status Report #2786
Changes from 2 commits
b5d0d5b
a6cdbd1
6df44c9
52ec23d
d2ba18f
ac9ee9d
25d63d7
ea8cf03
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed that this bloc of code repeated the third time. Could we move it to a separate function?
I am implementing
publish
command and will repeat it a fourth time. For sure I could do it in my PR, just curious is it the right way of refactoring, or is there some other sense in repeating the same code several times?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be a function or could be before the main
switch(event)
injanus_sip_sofia_callback
. Auth and ProxyAuth are not used outside of this function, IMHO it is better to split code into functions only if used elsewhere.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do re-use the same auth code for different SIP responses, true. Anyway, this isn't something I'd do here: this is a change I'd do separately in another PR, devoted just to that, so I can look into this once we merge this effort.