You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Alexey Varlamov <al...@gmail.com> on 2006/10/03 12:19:45 UTC
[r451637] - Code cleanup - ... - Remove unnecessary comments
Nathan,
I've seen you dropped many TODOs in "- Code cleanup -" series of commits;
I'd like to know what reasoning was behind this? I think it's a bit
early to erase TODOs without appropriate consideration...
In particular, could you please undo the following change, it produces
garbage messages during AUTH testing:
modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
===========
@@ -216,12 +206,12 @@ public class DefaultConfigurationParser
try {
val = PolicyUtils.expand(st.sval, system);
} catch (Exception e) {
- //TODO: warning log
- }
+ e.printStackTrace();
+ }
}
--
WBR,
Alexey
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Alexei Zakharov <al...@gmail.com>.
> I'd say leave the TODOs alone, at least until we're in a phase where
> such polishing up is desired.
Agreed. I've already posted a lot of
// XXX investigate
messages to myself to be read in the future.
Regards,
2006/10/4, Alex Blewitt <al...@gmail.com>:
> I use TODOs a lot in my code to remind me to come back to that
> particular piece and do the job properly. If someone else were to
> remove them then they may not do the right thing as far as the code
> needs ... so I'd expect at least some kind of heads-up before this
> would happen :-)
>
> I'd say leave the TODOs alone, at least until we're in a phase where
> such polishing up is desired.
>
> Alex.
>
> On 04/10/06, Nathan Beyer <nb...@gmail.com> wrote:
> > If this is an event that should be logged, as the TODO indicated, then
> > why not just print out the stack trace and be done with it? If this
> > exception happens so often that you'd like it removed, then why would
> > we want to log a warning message, which I would presume would print to
> > the console just as frequently.
> >
> > As for TODOs, in general I find TODOs never get done, especially
> > trivial ones like this particular case.
> >
> > -Nathan
> >
> > On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
> > > Nathan,
> > >
> > > I've seen you dropped many TODOs in "- Code cleanup -" series of commits;
> > > I'd like to know what reasoning was behind this? I think it's a bit
> > > early to erase TODOs without appropriate consideration...
> > >
> > > In particular, could you please undo the following change, it produces
> > > garbage messages during AUTH testing:
> > >
> > > modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
> > > ===========
> > > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
> > > try {
> > > val = PolicyUtils.expand(st.sval, system);
> > > } catch (Exception e) {
> > > - //TODO: warning log
> > > - }
> > > + e.printStackTrace();
> > > + }
> > > }
> > >
> > > --
> > > WBR,
> > > Alexey
> > >
> > > ---------------------------------------------------------------------
> > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>
--
Alexei Zakharov,
Intel Middleware Product Division
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Tim Ellison <t....@gmail.com>.
Nathan Beyer wrote:
>> From: Tim Ellison [mailto:t.p.ellison@gmail.com]
>>
>> Leave them in unless you put them in or are fixing it.
>
> I did put in a fix; I replaced the TODO with the stack trace print. It is my
> OPINION that TODO comments are, in general (80%), crap, but I've only
> replaced TODOs in Harmony with actual code or documentation.
Sure -- I wasn't referring to this particular occasion, I was espousing
a guideline on existing TODO's.
Regards,
Tim
--
Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
RE: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Nathan Beyer <nb...@gmail.com>.
> -----Original Message-----
> From: Tim Ellison [mailto:t.p.ellison@gmail.com]
> Sent: Wednesday, October 04, 2006 11:17 AM
> To: harmony-dev@incubator.apache.org
> Subject: Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
>
> Alex Blewitt wrote:
> > I use TODOs a lot in my code to remind me to come back to that
> > particular piece and do the job properly. If someone else were to
> > remove them then they may not do the right thing as far as the code
> > needs ... so I'd expect at least some kind of heads-up before this
> > would happen :-)
> >
> > I'd say leave the TODOs alone, at least until we're in a phase where
> > such polishing up is desired.
>
> +1
>
> Leave them in unless you put them in or are fixing it.
>
> Regards,
> Tim
I did put in a fix; I replaced the TODO with the stack trace print. It is my
OPINION that TODO comments are, in general (80%), crap, but I've only
replaced TODOs in Harmony with actual code or documentation.
-Nathan
>
> > On 04/10/06, Nathan Beyer <nb...@gmail.com> wrote:
> >> If this is an event that should be logged, as the TODO indicated, then
> >> why not just print out the stack trace and be done with it? If this
> >> exception happens so often that you'd like it removed, then why would
> >> we want to log a warning message, which I would presume would print to
> >> the console just as frequently.
> >>
> >> As for TODOs, in general I find TODOs never get done, especially
> >> trivial ones like this particular case.
> >>
> >> -Nathan
> >>
> >> On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
> >> > Nathan,
> >> >
> >> > I've seen you dropped many TODOs in "- Code cleanup -" series of
> >> commits;
> >> > I'd like to know what reasoning was behind this? I think it's a bit
> >> > early to erase TODOs without appropriate consideration...
> >> >
> >> > In particular, could you please undo the following change, it
> produces
> >> > garbage messages during AUTH testing:
> >> >
> >> >
> >>
> modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultCon
> figurationParser.java
> >>
> >> > ===========
> >> > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
> >> > try {
> >> > val = PolicyUtils.expand(st.sval, system);
> >> > } catch (Exception e) {
> >> > - //TODO: warning log
> >> > - }
> >> > + e.printStackTrace();
> >> > + }
> >> > }
> >> >
> >> > --
> >> > WBR,
> >> > Alexey
> >> >
> >> > ---------------------------------------------------------------------
> >> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> >> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >> > For additional commands, e-mail: harmony-dev-
> help@incubator.apache.org
> >> >
> >> >
> >>
> >> ---------------------------------------------------------------------
> >> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >>
> >>
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
> --
>
> Tim Ellison (t.p.ellison@gmail.com)
> IBM Java technology centre, UK.
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Tim Ellison <t....@gmail.com>.
Alex Blewitt wrote:
> I use TODOs a lot in my code to remind me to come back to that
> particular piece and do the job properly. If someone else were to
> remove them then they may not do the right thing as far as the code
> needs ... so I'd expect at least some kind of heads-up before this
> would happen :-)
>
> I'd say leave the TODOs alone, at least until we're in a phase where
> such polishing up is desired.
+1
Leave them in unless you put them in or are fixing it.
Regards,
Tim
> On 04/10/06, Nathan Beyer <nb...@gmail.com> wrote:
>> If this is an event that should be logged, as the TODO indicated, then
>> why not just print out the stack trace and be done with it? If this
>> exception happens so often that you'd like it removed, then why would
>> we want to log a warning message, which I would presume would print to
>> the console just as frequently.
>>
>> As for TODOs, in general I find TODOs never get done, especially
>> trivial ones like this particular case.
>>
>> -Nathan
>>
>> On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
>> > Nathan,
>> >
>> > I've seen you dropped many TODOs in "- Code cleanup -" series of
>> commits;
>> > I'd like to know what reasoning was behind this? I think it's a bit
>> > early to erase TODOs without appropriate consideration...
>> >
>> > In particular, could you please undo the following change, it produces
>> > garbage messages during AUTH testing:
>> >
>> >
>> modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
>>
>> > ===========
>> > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
>> > try {
>> > val = PolicyUtils.expand(st.sval, system);
>> > } catch (Exception e) {
>> > - //TODO: warning log
>> > - }
>> > + e.printStackTrace();
>> > + }
>> > }
>> >
>> > --
>> > WBR,
>> > Alexey
>> >
>> > ---------------------------------------------------------------------
>> > Terms of use : http://incubator.apache.org/harmony/mailing.html
>> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>> >
>> >
>>
>> ---------------------------------------------------------------------
>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>
--
Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Alex Blewitt <al...@gmail.com>.
I use TODOs a lot in my code to remind me to come back to that
particular piece and do the job properly. If someone else were to
remove them then they may not do the right thing as far as the code
needs ... so I'd expect at least some kind of heads-up before this
would happen :-)
I'd say leave the TODOs alone, at least until we're in a phase where
such polishing up is desired.
Alex.
On 04/10/06, Nathan Beyer <nb...@gmail.com> wrote:
> If this is an event that should be logged, as the TODO indicated, then
> why not just print out the stack trace and be done with it? If this
> exception happens so often that you'd like it removed, then why would
> we want to log a warning message, which I would presume would print to
> the console just as frequently.
>
> As for TODOs, in general I find TODOs never get done, especially
> trivial ones like this particular case.
>
> -Nathan
>
> On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
> > Nathan,
> >
> > I've seen you dropped many TODOs in "- Code cleanup -" series of commits;
> > I'd like to know what reasoning was behind this? I think it's a bit
> > early to erase TODOs without appropriate consideration...
> >
> > In particular, could you please undo the following change, it produces
> > garbage messages during AUTH testing:
> >
> > modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
> > ===========
> > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
> > try {
> > val = PolicyUtils.expand(st.sval, system);
> > } catch (Exception e) {
> > - //TODO: warning log
> > - }
> > + e.printStackTrace();
> > + }
> > }
> >
> > --
> > WBR,
> > Alexey
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Oliver Deakin <ol...@googlemail.com>.
Geir Magnusson Jr. wrote:
> Stepan Mishura wrote:
>> On 10/4/06, Nathan Beyer wrote:
>>>
>>> If this is an event that should be logged, as the TODO indicated, then
>>> why not just print out the stack trace and be done with it? If this
>>> exception happens so often that you'd like it removed, then why would
>>> we want to log a warning message, which I would presume would print to
>>> the console just as frequently.
>>>
>>> As for TODOs, in general I find TODOs never get done, especially
>>> trivial ones like this particular case.
>>
>>
>>
>> I don't agree. TODOs are good hint for making improvements and I'd
>> keep them
>> in source files.
>
> Me too. I don't like cruft, but I'm not sure I see the harm in general.
Agreed - Id prefer to keep the TODOs in there. They act as pointers for
bits of
work that need to be done, and can be helpful to people looking to make
small
contributions of effort to the project.
Regards,
Oliver
>
> geir
>
>>
>> Thanks,
>> Stepan.
>>
>> -Nathan
>>>
>>> On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
>>> > Nathan,
>>> >
>>> > I've seen you dropped many TODOs in "- Code cleanup -" series of
>>> commits;
>>> > I'd like to know what reasoning was behind this? I think it's a bit
>>> > early to erase TODOs without appropriate consideration...
>>> >
>>> > In particular, could you please undo the following change, it
>>> produces
>>> > garbage messages during AUTH testing:
>>> >
>>> >
>>> modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
>>>
>>> > ===========
>>> > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
>>> > try {
>>> > val = PolicyUtils.expand(st.sval, system);
>>> > } catch (Exception e) {
>>> > - //TODO: warning log
>>> > - }
>>> > + e.printStackTrace();
>>> > + }
>>> > }
>>> >
>>> > --
>>> > WBR,
>>> > Alexey
>>>
>>>
>> ------------------------------------------------------
>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>>
>
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>
--
Oliver Deakin
IBM United Kingdom Limited
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by "Geir Magnusson Jr." <ge...@pobox.com>.
Stepan Mishura wrote:
> On 10/4/06, Nathan Beyer wrote:
>>
>> If this is an event that should be logged, as the TODO indicated, then
>> why not just print out the stack trace and be done with it? If this
>> exception happens so often that you'd like it removed, then why would
>> we want to log a warning message, which I would presume would print to
>> the console just as frequently.
>>
>> As for TODOs, in general I find TODOs never get done, especially
>> trivial ones like this particular case.
>
>
>
> I don't agree. TODOs are good hint for making improvements and I'd keep
> them
> in source files.
Me too. I don't like cruft, but I'm not sure I see the harm in general.
geir
>
> Thanks,
> Stepan.
>
> -Nathan
>>
>> On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
>> > Nathan,
>> >
>> > I've seen you dropped many TODOs in "- Code cleanup -" series of
>> commits;
>> > I'd like to know what reasoning was behind this? I think it's a bit
>> > early to erase TODOs without appropriate consideration...
>> >
>> > In particular, could you please undo the following change, it produces
>> > garbage messages during AUTH testing:
>> >
>> >
>> modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
>>
>> > ===========
>> > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
>> > try {
>> > val = PolicyUtils.expand(st.sval, system);
>> > } catch (Exception e) {
>> > - //TODO: warning log
>> > - }
>> > + e.printStackTrace();
>> > + }
>> > }
>> >
>> > --
>> > WBR,
>> > Alexey
>>
>>
> ------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Stepan Mishura <st...@gmail.com>.
On 10/4/06, Nathan Beyer wrote:
>
> If this is an event that should be logged, as the TODO indicated, then
> why not just print out the stack trace and be done with it? If this
> exception happens so often that you'd like it removed, then why would
> we want to log a warning message, which I would presume would print to
> the console just as frequently.
>
> As for TODOs, in general I find TODOs never get done, especially
> trivial ones like this particular case.
I don't agree. TODOs are good hint for making improvements and I'd keep them
in source files.
Thanks,
Stepan.
-Nathan
>
> On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
> > Nathan,
> >
> > I've seen you dropped many TODOs in "- Code cleanup -" series of
> commits;
> > I'd like to know what reasoning was behind this? I think it's a bit
> > early to erase TODOs without appropriate consideration...
> >
> > In particular, could you please undo the following change, it produces
> > garbage messages during AUTH testing:
> >
> >
> modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
> > ===========
> > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
> > try {
> > val = PolicyUtils.expand(st.sval, system);
> > } catch (Exception e) {
> > - //TODO: warning log
> > - }
> > + e.printStackTrace();
> > + }
> > }
> >
> > --
> > WBR,
> > Alexey
>
>
------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
RE: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Nathan Beyer <nb...@gmail.com>.
> -----Original Message-----
> From: Alexey Varlamov [mailto:alexey.v.varlamov@gmail.com]
> Sent: Wednesday, October 04, 2006 5:13 AM
> To: harmony-dev@incubator.apache.org
> Subject: Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
>
> 2006/10/4, Nathan Beyer <nb...@gmail.com>:
> > If this is an event that should be logged, as the TODO indicated, then
> > why not just print out the stack trace and be done with it? If this
> > exception happens so often that you'd like it removed, then why would
> > we want to log a warning message, which I would presume would print to
> > the console just as frequently.
>
> Logging and printing to console essentially differ in flexible
> customization offered by the first approach. An application can have
> no console after all.
> We develop the common class library here, not an ordinary application
> - so let's not assume it will be used in some particular way.
Every JRE that's I've worked with since Java SE 1.4, distributes a default
configuration with the logging level set to INFO, so logging a warning, as
the TODO indicated, would write out just as frequently as printing a stack
trace. I wasn't assuming a particular use, I was assuming the default, out
of the box use.
-Nathan
>
> In this concrete case, stack trace is printed every time invalid input
> data is supplied - and it is normal for a unit test to cover some
> negative cases.
> But seeing console jammed with that rubbish is quite confusing (for me
> at least). OTOH, such info would be valuable for app developer - so
> why not satisfy both?
>
> >
> > As for TODOs, in general I find TODOs never get done, especially
> > trivial ones like this particular case.
> Because we have more priority tasks to be done. We just haven't
> reached that stage of completeness, when we can afford minor refining
> and polishing. Never say never, you know :)
>
> Please, announce ahead if you do such things in future.
>
> --
> Regards,
> Alexey
>
> >
> > -Nathan
> >
> > On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
> > > Nathan,
> > >
> > > I've seen you dropped many TODOs in "- Code cleanup -" series of
> commits;
> > > I'd like to know what reasoning was behind this? I think it's a bit
> > > early to erase TODOs without appropriate consideration...
> > >
> > > In particular, could you please undo the following change, it produces
> > > garbage messages during AUTH testing:
> > >
> > >
> modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultCon
> figurationParser.java
> > > ===========
> > > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
> > > try {
> > > val = PolicyUtils.expand(st.sval, system);
> > > } catch (Exception e) {
> > > - //TODO: warning log
> > > - }
> > > + e.printStackTrace();
> > > + }
> > > }
> > >
> > > --
> > > WBR,
> > > Alexey
> > >
> > > ---------------------------------------------------------------------
> > > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Alexey Varlamov <al...@gmail.com>.
2006/10/4, Nathan Beyer <nb...@gmail.com>:
> If this is an event that should be logged, as the TODO indicated, then
> why not just print out the stack trace and be done with it? If this
> exception happens so often that you'd like it removed, then why would
> we want to log a warning message, which I would presume would print to
> the console just as frequently.
Logging and printing to console essentially differ in flexible
customization offered by the first approach. An application can have
no console after all.
We develop the common class library here, not an ordinary application
- so let's not assume it will be used in some particular way.
In this concrete case, stack trace is printed every time invalid input
data is supplied - and it is normal for a unit test to cover some
negative cases.
But seeing console jammed with that rubbish is quite confusing (for me
at least). OTOH, such info would be valuable for app developer - so
why not satisfy both?
>
> As for TODOs, in general I find TODOs never get done, especially
> trivial ones like this particular case.
Because we have more priority tasks to be done. We just haven't
reached that stage of completeness, when we can afford minor refining
and polishing. Never say never, you know :)
Please, announce ahead if you do such things in future.
--
Regards,
Alexey
>
> -Nathan
>
> On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
> > Nathan,
> >
> > I've seen you dropped many TODOs in "- Code cleanup -" series of commits;
> > I'd like to know what reasoning was behind this? I think it's a bit
> > early to erase TODOs without appropriate consideration...
> >
> > In particular, could you please undo the following change, it produces
> > garbage messages during AUTH testing:
> >
> > modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
> > ===========
> > @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
> > try {
> > val = PolicyUtils.expand(st.sval, system);
> > } catch (Exception e) {
> > - //TODO: warning log
> > - }
> > + e.printStackTrace();
> > + }
> > }
> >
> > --
> > WBR,
> > Alexey
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [r451637] - Code cleanup - ... - Remove unnecessary comments
Posted by Nathan Beyer <nb...@gmail.com>.
If this is an event that should be logged, as the TODO indicated, then
why not just print out the stack trace and be done with it? If this
exception happens so often that you'd like it removed, then why would
we want to log a warning message, which I would presume would print to
the console just as frequently.
As for TODOs, in general I find TODOs never get done, especially
trivial ones like this particular case.
-Nathan
On 10/3/06, Alexey Varlamov <al...@gmail.com> wrote:
> Nathan,
>
> I've seen you dropped many TODOs in "- Code cleanup -" series of commits;
> I'd like to know what reasoning was behind this? I think it's a bit
> early to erase TODOs without appropriate consideration...
>
> In particular, could you please undo the following change, it produces
> garbage messages during AUTH testing:
>
> modules/auth/src/main/java/common/org/apache/harmony/auth/login/DefaultConfigurationParser.java
> ===========
> @@ -216,12 +206,12 @@ public class DefaultConfigurationParser
> try {
> val = PolicyUtils.expand(st.sval, system);
> } catch (Exception e) {
> - //TODO: warning log
> - }
> + e.printStackTrace();
> + }
> }
>
> --
> WBR,
> Alexey
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>
---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org