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

LaborEtArs's mDNS fixed #5384

Closed
wants to merge 26 commits into from
Closed

LaborEtArs's mDNS fixed #5384

wants to merge 26 commits into from

Conversation

d-a-v
Copy link
Collaborator

@d-a-v d-a-v commented Nov 28, 2018

The two examples work well. I did not make further testing.

(may replace #5370)

@d-a-v
Copy link
Collaborator Author

d-a-v commented Nov 28, 2018

@LaborEtArs
Either you fix your PR, or we can merge this one with the github co-author feature (this is the only way when using squash&merge to my knowledge) (once approved).

@d-a-v d-a-v requested a review from devyte November 28, 2018 11:50
@d-a-v d-a-v added this to the 2.5.0 milestone Nov 28, 2018
@devyte devyte mentioned this pull request Dec 5, 2018
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

There are still a few compat method overloads missing as well as some cleanup and formalization work needed, among other minor details, but overall this is just amazing work. Techniques are a bit old fashioned for modern C++, and yet the code structure and style discipline are so good that it makes it irrelevant.
Congratulations @LaborEtArs ! Approved.

arihantdaga and others added 4 commits December 5, 2018 16:24
DEBUG_OUTPUT needs to go inside DEBUG_EX_ function otherwise throw error if DEBUG_ESP_MDNS_RESPONDER is not defined
Disable DEBUG_ESP_MDNS_RESPONDER by default
@d-a-v
Copy link
Collaborator Author

d-a-v commented Dec 5, 2018

closing in favour of #5442

@d-a-v d-a-v closed this Dec 5, 2018
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.

4 participants