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

Added cosmetix from the pre-group-refax #1071

Merged

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Jan 21, 2020

Recommended turning off whitespace difs for review.

@ethouris ethouris added Priority: High Type: Maintenance Work required to maintain or clean up the code [core] Area: Changes in SRT library core Impact: Low labels Jan 21, 2020
@ethouris ethouris self-assigned this Jan 21, 2020
Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

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

  • Let's extract the renaming of UDP_EPOLL_IN -> SRT_EPOLL_IN etc. into a small PR, so that we get 1 commit in master.

srtcore/api.cpp Outdated
Comment on lines 330 to 332
// connection already exist, this is a repeated connection request
// respond with existing HS information
// respond with existing HS information
HLOGC(mglog.Debug, log
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alignment

@ethouris
Copy link
Collaborator Author

ethouris commented Jan 21, 2020

  • Let's extract the renaming of UDP_EPOLL_IN -> SRT_EPOLL_IN etc. into a small PR, so that we get 1 commit in master.

Alright. So hold horses until I make this one.

@maxsharabayko maxsharabayko added this to the v1.5.0 milestone Jan 21, 2020
@ethouris
Copy link
Collaborator Author

#1077 is handling the renames.

@maxsharabayko
Copy link
Collaborator

#1077 is merged - updated this PR

Comment on lines 73 to 74
// Use "inline namespace" in C++11
namespace srt_logging
Copy link
Collaborator

Choose a reason for hiding this comment

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

No point in this comment. For very distant future?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Likely. Old times. I'll remove it, it's useless anyway.

srtcore/packet.h Outdated
Comment on lines 421 to 422
std::string MessageFlagStr() { return ""; }
std::string Info() { return ""; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

return std::string() to avoid implicit conversions?

@maxsharabayko
Copy link
Collaborator

The build needs to be fixed

/home/travis/build/Haivision/srt/srtcore/packet.h:422:32: error: expected primary-expression before ‘return’
    std::string Info() { return return std::string(); }

@maxsharabayko maxsharabayko merged commit a080a3f into Haivision:master Jan 22, 2020
@mbakholdina mbakholdina modified the milestones: v1.5.0, v1.4.2 Oct 14, 2020
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 Priority: High Type: Maintenance Work required to maintain or clean up the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants