You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@turbine.apache.org by EP...@upstate.com on 2003/06/07 10:30:45 UTC
RE: cvs commit: jakarta-turbine-2/src/java/org/apache/turbine/uti
l TurbineConfig.java
Good point.. What keeps catching me on the line formatting is Eclipse.. I
get into the habbit of hitting control-F, and while I don't mean to do it
with my Turbine code, I occasionally do it.
I am not sure how to do a rollback, but it seems that since your
TurbineXmlConfig idea is good, that might be the easiest path? Do you
perform a rollback by getting the old code out and rechecking it in?
I did think the funny logic on the extension was crummy, but I often don't
think about actually adding more classes, although that would be the right
way... I will add that as another code "smell" to look for.
Eric
-----Original Message-----
From: Henning P. Schmiedehausen [mailto:hps@intermeta.de]
Sent: Saturday, June 07, 2003 4:26 AM
To: turbine-dev@jakarta.apache.org
Subject: Re: cvs commit:
jakarta-turbine-2/src/java/org/apache/turbine/util TurbineConfig.java
epugh@apache.org writes:
Eric,
can you please stop reformatting lines without good reason? We agreed to
be somehow agnostic to line wrapping as long as they pass through checkstyle
without error. As far as I can see, all of these
public class Foo implement Bar, Baz
{
}
public class Foo
implement Bar, Baz
{
}
public class Foo
implement Bar,
Baz
{
}
do. This one don't:
> -public class TurbineConfig
> - implements ServletConfig, ServletContext, Initializable,
Disposable
> +public class TurbineConfig implements ServletConfig, ServletContext,
Initializable, Disposable
We are running checkstyle with linewrap == 80. So these lines will now
show up. And we're adding unneccesary changes to the code which make
it hard to track in the annotation view, who changed what.
> - initParams.put(PROPERTIES_PATH_KEY, properties);
> + if (properties.toLowerCase().endsWith(".xml"))
> + {
> + initParams.put(CONFIGURATION_PATH_KEY, properties);
> + }
> + else
> + {
> + initParams.put(PROPERTIES_PATH_KEY, properties);
> + }
> }
I actually _hate_ checks like these. They lead to unplasant
surprises. What if we get a third kind of configuration? Will we add
more checks?
Please make a TurbineXmlConfig which is derived from TurbineConfig and
accepts an XML file. This shouldn't be too hard. If someone is using
the standalone Turbine, he should be able to use the right
configuration class.
Regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
Java, perl, Solaris, Linux, xSP Consulting, Web Services
freelance consultant -- Jakarta Turbine Development -- hero for hire
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-turbine-2/src/java/org/apache/turbine/uti
Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
EPugh@upstate.com writes:
>I am not sure how to do a rollback, but it seems that since your
>TurbineXmlConfig idea is good, that might be the easiest path? Do you
>perform a rollback by getting the old code out and rechecking it in?
Just did so. The current head should be back on 1.13.
>I did think the funny logic on the extension was crummy, but I often don't
>think about actually adding more classes, although that would be the right
>way... I will add that as another code "smell" to look for.
Thanks a lot.
Regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
Java, perl, Solaris, Linux, xSP Consulting, Web Services
freelance consultant -- Jakarta Turbine Development -- hero for hire
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-turbine-2/src/java/org/apache/turbine/util TurbineConfig.java
Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
"Quinton McCombs" <qm...@nequalsone.com> writes:
>In the future, if you need to rollback after a commit, just check out the
>previous version and then copy/paste over the current version and check it
>back in.
You can't do that, because if you check out a previous version with -r, you
will get a "sticky" file. You can do a checkout with
cvs checkout -r<old_version> -p <botched_file> > <botched_file>
to avoid this.
[...]
>I actually have the same habit of always format the code and optimize
>imports. What keeps me from making the same mistakes is that I _always_ do
>a cvs diff before I commit anything. An added benefit to this step is that
>you have one final chance to make sure that your commit message covers all
>of the changes. This is a very good habit to have.
Same here. :-)
Eric, BTW: Shouldn't this be called TurbineXMLConfig instead of
TurbineXmlConfig ? I remember the discussion about acronyms and even
Sun did deprecate some of the Ucase in favour of UCASE.
Regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
Java, perl, Solaris, Linux, xSP Consulting, Web Services
freelance consultant -- Jakarta Turbine Development -- hero for hire
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
RE: cvs commit: jakarta-turbine-2/src/java/org/apache/turbine/util TurbineConfig.java
Posted by Quinton McCombs <qm...@nequalsone.com>.
In the future, if you need to rollback after a commit, just check out the
previous version and then copy/paste over the current version and check it
back in.
> -----Original Message-----
> From: EPugh@upstate.com [mailto:EPugh@upstate.com]
> Sent: Saturday, June 07, 2003 3:31 AM
> To: turbine-dev@jakarta.apache.org
> Subject: RE: cvs commit:
> jakarta-turbine-2/src/java/org/apache/turbine/util TurbineConfig.java
>
>
> Good point.. What keeps catching me on the line formatting
> is Eclipse.. I get into the habbit of hitting control-F, and
> while I don't mean to do it with my Turbine code, I
> occasionally do it.
>
I actually have the same habit of always format the code and optimize
imports. What keeps me from making the same mistakes is that I _always_ do
a cvs diff before I commit anything. An added benefit to this step is that
you have one final chance to make sure that your commit message covers all
of the changes. This is a very good habit to have.
> I am not sure how to do a rollback, but it seems that since
> your TurbineXmlConfig idea is good, that might be the easiest
> path? Do you perform a rollback by getting the old code out
> and rechecking it in?
>
You are correct. Check out the previous version and then copy/paste over
the current version and check it back in.
> I did think the funny logic on the extension was crummy, but
> I often don't think about actually adding more classes,
> although that would be the right way... I will add that as
> another code "smell" to look for.
>
In general, this type of conditional logic can and should be replaced with
inheritance. It certainly is a distinctive "smell". :-)
> Eric
>
> -----Original Message-----
> From: Henning P. Schmiedehausen [mailto:hps@intermeta.de]
> Sent: Saturday, June 07, 2003 4:26 AM
> To: turbine-dev@jakarta.apache.org
> Subject: Re: cvs commit:
> jakarta-turbine-2/src/java/org/apache/turbine/util TurbineConfig.java
>
>
> epugh@apache.org writes:
>
> Eric,
>
> can you please stop reformatting lines without good reason?
> We agreed to be somehow agnostic to line wrapping as long as
> they pass through checkstyle without error. As far as I can
> see, all of these
>
> public class Foo implement Bar, Baz
> {
> }
>
> public class Foo
> implement Bar, Baz
> {
> }
>
> public class Foo
> implement Bar,
> Baz
> {
> }
>
> do. This one don't:
>
> > -public class TurbineConfig
> > - implements ServletConfig, ServletContext, Initializable,
> Disposable
> > +public class TurbineConfig implements ServletConfig,
> ServletContext,
> Initializable, Disposable
>
> We are running checkstyle with linewrap == 80. So these lines
> will now show up. And we're adding unneccesary changes to the
> code which make it hard to track in the annotation view, who
> changed what.
>
> > - initParams.put(PROPERTIES_PATH_KEY, properties);
> > + if (properties.toLowerCase().endsWith(".xml"))
> > + {
> > + initParams.put(CONFIGURATION_PATH_KEY, properties);
> > + }
> > + else
> > + {
> > + initParams.put(PROPERTIES_PATH_KEY, properties);
> > + }
> > }
>
> I actually _hate_ checks like these. They lead to unplasant
> surprises. What if we get a third kind of configuration? Will
> we add more checks?
>
> Please make a TurbineXmlConfig which is derived from
> TurbineConfig and accepts an XML file. This shouldn't be too
> hard. If someone is using the standalone Turbine, he should
> be able to use the right configuration class.
>
> Regards
> Henning
>
> --
> Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
> hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
>
> Java, perl, Solaris, Linux, xSP Consulting, Web Services
> freelance consultant -- Jakarta Turbine Development -- hero for hire
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: turbine-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org