|
|
Created:
7 years, 6 months ago by yael.aharon Modified:
7 years, 6 months ago CC:
chromium-reviews, native-client-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionRemove the switches kSilentDumpOnDCHECK and kMemoryProfiling from NaCl and broker processes.
The reason for removing these flags is because they seem to not be used, and they cause a problem in moving NaCl code to components, because they are defined in the chrome layer.
This is in preparation to move NaCl code to components.
BUG=244791
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207914
Patch Set 1 #
Messages
Total messages: 18 (0 generated)
This is broken off from https://codereview.chromium.org/16881004/ Can you please review?
LGTM. Is there a particular reason to remove these two? I mean, the list of flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, but I don't know if it would hurt to propagate flags that aren't used. BTW, when you post a review for the first time, please use "Publish+Mail Comments" rather than "Reply". Then Rietveld will send an e-mail with more details about the change.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yael.aharon@intel.com/17041003/1
On 2013/06/21 01:07:39, Mark Seaborn wrote: > LGTM. Is there a particular reason to remove these two? I mean, the list of > flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, but I > don't know if it would hurt to propagate flags that aren't used. > > BTW, when you post a review for the first time, please use "Publish+Mail > Comments" rather than "Reply". Then Rietveld will send an e-mail with more > details about the change. Sorry :)
On 2013/06/21 01:07:39, Mark Seaborn wrote: > LGTM. Is there a particular reason to remove these two? I mean, the list of > flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, but I > don't know if it would hurt to propagate flags that aren't used. > The reason for removing these flags is because they seem to not be used, and they cause a problem in moving NaCl code to components, because they are defined in the chrome layer.
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...
jam@ could you please rubber stamp?
On 20 June 2013 19:20, <yael.aharon@chromium.org> wrote: > On 2013/06/21 01:07:39, Mark Seaborn wrote: > >> LGTM. Is there a particular reason to remove these two? I mean, the >> list of >> flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, >> but I >> > don't know if it would hurt to propagate flags that aren't used. >> > > The reason for removing these flags is because they seem to not be used, > and > they cause a problem in moving NaCl code to components, because they are > defined > in the chrome layer. > OK. Can you put that explanation in the commit message, please? Cheers, Mark -- You received this message because you are subscribed to the Google Groups "Native-Client-Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to native-client-reviews+unsubscribe@googlegroups.com. To post to this group, send email to native-client-reviews@googlegroups.com. Visit this group at http://groups.google.com/group/native-client-reviews. For more options, visit https://groups.google.com/groups/opt_out.
On 20 June 2013 19:20, <yael.aharon@chromium.org> wrote: > On 2013/06/21 01:07:39, Mark Seaborn wrote: > >> LGTM. Is there a particular reason to remove these two? I mean, the >> list of >> flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, >> but I >> > don't know if it would hurt to propagate flags that aren't used. >> > > The reason for removing these flags is because they seem to not be used, > and > they cause a problem in moving NaCl code to components, because they are > defined > in the chrome layer. > OK. Can you put that explanation in the commit message, please? Cheers, Mark
On 2013/06/21 02:25:14, Mark Seaborn wrote: > On 20 June 2013 19:20, <mailto:yael.aharon@chromium.org> wrote: > > > On 2013/06/21 01:07:39, Mark Seaborn wrote: > > > >> LGTM. Is there a particular reason to remove these two? I mean, the > >> list of > >> flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, > >> but I > >> > > don't know if it would hurt to propagate flags that aren't used. > >> > > > > The reason for removing these flags is because they seem to not be used, > > and > > they cause a problem in moving NaCl code to components, because they are > > defined > > in the chrome layer. > > > > OK. Can you put that explanation in the commit message, please? > > Cheers, > Mark Done!
On 20 June 2013 19:10, <yael.aharon@chromium.org> wrote: > On 2013/06/21 01:07:39, Mark Seaborn wrote: > >> LGTM. Is there a particular reason to remove these two? I mean, the >> list of >> flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, >> but I >> > don't know if it would hurt to propagate flags that aren't used. >> > > BTW, when you post a review for the first time, please use "Publish+Mail >> Comments" rather than "Reply". Then Rietveld will send an e-mail with >> more >> details about the change. >> > > Sorry :) > No worries! Also, "Publish+Mail Comments" only has that effect if you use it from the same account that you used "git cl upload" with. So you might want to log into the web interface with your @chromium.org account rather than your @intel.com account. I guess that's why the initial e-mail isn't working, rather than coming from using "Reply". It's a pity Rietveld has so many of these quirks. Cheers, Mark
On 20 June 2013 19:10, <yael.aharon@chromium.org> wrote: > On 2013/06/21 01:07:39, Mark Seaborn wrote: > >> LGTM. Is there a particular reason to remove these two? I mean, the >> list of >> flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, >> but I >> > don't know if it would hurt to propagate flags that aren't used. >> > > BTW, when you post a review for the first time, please use "Publish+Mail >> Comments" rather than "Reply". Then Rietveld will send an e-mail with >> more >> details about the change. >> > > Sorry :) > No worries! Also, "Publish+Mail Comments" only has that effect if you use it from the same account that you used "git cl upload" with. So you might want to log into the web interface with your @chromium.org account rather than your @intel.com account. I guess that's why the initial e-mail isn't working, rather than coming from using "Reply". It's a pity Rietveld has so many of these quirks. Cheers, Mark -- You received this message because you are subscribed to the Google Groups "Native-Client-Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to native-client-reviews+unsubscribe@googlegroups.com. To post to this group, send email to native-client-reviews@googlegroups.com. Visit this group at http://groups.google.com/group/native-client-reviews. For more options, visit https://groups.google.com/groups/opt_out.
I have to alternate the accounts because my company policy requires me to submit patches with @intel.com, and codereview.google.com is very restrictive if I don't use a @chromium.org account. My colleagues filed bugs about this, but no solution so far. From: mseaborn@google.com [mailto:mseaborn@google.com] On Behalf Of Mark Seaborn Sent: Friday, June 21, 2013 11:51 AM To: Aharon, Yael; mseaborn@chromium.org; jam@chromium.org; yael.aharon@chromium.org; chromium-reviews@chromium.org; native-client-reviews@googlegroups.com Subject: Re: Remove the switches kSilentDumpOnDCHECK and kMemoryProfiling from NaCl and broker processes. (issue 17041003) On 20 June 2013 19:10, <yael.aharon@chromium.org<mailto:yael.aharon@chromium.org>> wrote: On 2013/06/21 01:07:39, Mark Seaborn wrote: LGTM. Is there a particular reason to remove these two? I mean, the list of flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, but I don't know if it would hurt to propagate flags that aren't used. BTW, when you post a review for the first time, please use "Publish+Mail Comments" rather than "Reply". Then Rietveld will send an e-mail with more details about the change. Sorry :) No worries! Also, "Publish+Mail Comments" only has that effect if you use it from the same account that you used "git cl upload" with. So you might want to log into the web interface with your @chromium.org<http://chromium.org> account rather than your @intel.com<http://intel.com> account. I guess that's why the initial e-mail isn't working, rather than coming from using "Reply". It's a pity Rietveld has so many of these quirks. Cheers, Mark -- You received this message because you are subscribed to the Google Groups "Native-Client-Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to native-client-reviews+unsubscribe@googlegroups.com. To post to this group, send email to native-client-reviews@googlegroups.com. Visit this group at http://groups.google.com/group/native-client-reviews. For more options, visit https://groups.google.com/groups/opt_out.
I have to alternate the accounts because my company policy requires me to submit patches with @intel.com, and codereview.google.com is very restrictive if I don't use a @chromium.org account. My colleagues filed bugs about this, but no solution so far. From: mseaborn@google.com [mailto:mseaborn@google.com] On Behalf Of Mark Seaborn Sent: Friday, June 21, 2013 11:51 AM To: Aharon, Yael; mseaborn@chromium.org; jam@chromium.org; yael.aharon@chromium.org; chromium-reviews@chromium.org; native-client-reviews@googlegroups.com Subject: Re: Remove the switches kSilentDumpOnDCHECK and kMemoryProfiling from NaCl and broker processes. (issue 17041003) On 20 June 2013 19:10, <yael.aharon@chromium.org<mailto:yael.aharon@chromium.org>> wrote: On 2013/06/21 01:07:39, Mark Seaborn wrote: LGTM. Is there a particular reason to remove these two? I mean, the list of flags seems fairly arbitrary. I don't know why kNoErrorDialogs is there, but I don't know if it would hurt to propagate flags that aren't used. BTW, when you post a review for the first time, please use "Publish+Mail Comments" rather than "Reply". Then Rietveld will send an e-mail with more details about the change. Sorry :) No worries! Also, "Publish+Mail Comments" only has that effect if you use it from the same account that you used "git cl upload" with. So you might want to log into the web interface with your @chromium.org<http://chromium.org> account rather than your @intel.com<http://intel.com> account. I guess that's why the initial e-mail isn't working, rather than coming from using "Reply". It's a pity Rietveld has so many of these quirks. Cheers, Mark
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yael.aharon@intel.com/17041003/1
Message was sent while issue was closed.
Change committed as 207914 |