Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
14188021:
Add enable_mdns flag (Closed)
|
Created:
7 years, 8 months ago by Noam Samuel Modified:
7 years, 8 months ago CC:
chromium-reviews, cbentzel+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionAdd enable_mdns flag
Add GYP flag to chrome for enabling mDNS support.
BUG=233821
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195874
Patch Set 1 #Patch Set 2 : Actually, no need to update net.gyp yet #
Total comments: 3
Patch Set 3 : Fix comment #Messages
Total messages: 14 (0 generated)
+mmenke +gene
Please add a bug for this general feature, and you can assign with this.
Just nits. https://codereview.chromium.org/14188021/diff/3001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1051 build/common.gypi:1051: # Enables MDNS support in chrome To be consistent with other comments, use # MDNS is disabled by default. https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1052 build/common.gypi:1052: 'enable_mdns%' : 0, Also - I wonder if this should be ENABLE_DNSSD or something similar since DNS-SD does work for non-MDNS cases as well (although mostly MDNS).
I was thinking separate, dependent flags for mDNS, DNS-SD, and any dependent services (privet/DNS-SD web services/etc). The main rationale for separate mDNS and DNS-SD flags is that mDNS can also be used to resolve .local domains. On 2013/04/18 17:41:01, cbentzel wrote: > Just nits. > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi > File build/common.gypi (right): > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1051 > build/common.gypi:1051: # Enables MDNS support in chrome > To be consistent with other comments, use > > # MDNS is disabled by default. > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1052 > build/common.gypi:1052: 'enable_mdns%' : 0, > Also - I wonder if this should be ENABLE_DNSSD or something similar since DNS-SD > does work for non-MDNS cases as well (although mostly MDNS).
> is that mDNS can also be used to resolve .local domains. Good point. On Thu, Apr 18, 2013 at 3:08 PM, <noamsml@chromium.org> wrote: > I was thinking separate, dependent flags for mDNS, DNS-SD, and any dependent > services (privet/DNS-SD web services/etc). The main rationale for separate > mDNS > and DNS-SD flags is that mDNS can also be used to resolve .local domains. > > > On 2013/04/18 17:41:01, cbentzel wrote: >> >> Just nits. > > >> https://codereview.chromium.org/14188021/diff/3001/build/common.gypi >> File build/common.gypi (right): > > > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1051 >> >> build/common.gypi:1051: # Enables MDNS support in chrome >> To be consistent with other comments, use > > >> # MDNS is disabled by default. > > > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1052 >> >> build/common.gypi:1052: 'enable_mdns%' : 0, >> Also - I wonder if this should be ENABLE_DNSSD or something similar since > > DNS-SD >> >> does work for non-MDNS cases as well (although mostly MDNS). > > > > > https://codereview.chromium.org/14188021/
https://codereview.chromium.org/14188021/diff/3001/build/common.gypi File build/common.gypi (right): https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1051 build/common.gypi:1051: # Enables MDNS support in chrome On 2013/04/18 17:41:02, cbentzel wrote: > To be consistent with other comments, use > > # MDNS is disabled by default. Done.
Does this look good? On 2013/04/19 22:24:12, Noam Samuel (chromium) wrote: > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi > File build/common.gypi (right): > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1051 > build/common.gypi:1051: # Enables MDNS support in chrome > On 2013/04/18 17:41:02, cbentzel wrote: > > To be consistent with other comments, use > > > > # MDNS is disabled by default. > > Done.
LGTM On 2013/04/22 18:06:25, Noam Samuel (chromium) wrote: > Does this look good? > > On 2013/04/19 22:24:12, Noam Samuel (chromium) wrote: > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi > > File build/common.gypi (right): > > > > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1051 > > build/common.gypi:1051: # Enables MDNS support in chrome > > On 2013/04/18 17:41:02, cbentzel wrote: > > > To be consistent with other comments, use > > > > > > # MDNS is disabled by default. > > > > Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noamsml@chromium.org/14188021/11001
Retried try job too often on win7_aura for step(s) interactive_ui_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/noamsml@chromium.org/14188021/11001
LGTM On Mon, Apr 22, 2013 at 5:45 PM, <mmenke@chromium.org> wrote: > LGTM > > > On 2013/04/22 18:06:25, Noam Samuel (chromium) wrote: >> >> Does this look good? > > >> On 2013/04/19 22:24:12, Noam Samuel (chromium) wrote: >> > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi >> > File build/common.gypi (right): >> > >> > > > > https://codereview.chromium.org/14188021/diff/3001/build/common.gypi#newcode1051 >> >> > build/common.gypi:1051: # Enables MDNS support in chrome >> > On 2013/04/18 17:41:02, cbentzel wrote: >> > > To be consistent with other comments, use >> > > >> > > # MDNS is disabled by default. >> > >> > Done. > > > > > https://codereview.chromium.org/14188021/
Message was sent while issue was closed.
Change committed as 195874 |
