commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Date Fri, 04 Oct 2013 20:58:08 GMT
Even for sandbox?
 Le 4 oct. 2013 22:12, "James Carman" <james@carmanconsulting.com> a ├ęcrit :

> This really is something that Sonar should catch for us.  I thought we
> had that turned on somewhere, right?
>
> On Fri, Oct 4, 2013 at 2:29 PM, sebb <sebbaz@gmail.com> wrote:
> > On 4 October 2013 18:04, Jean-Louis MONTEIRO <jeanouii@gmail.com> wrote:
> >> Apologize for the late answer.
> >> Not sure to understand the purpose of the request.
> >> Could you detail cause it's not used anywhere else?
> >
> > The code should not contain 'magic' strings (or numbers for that matter).
> >
> > All fixed strings should be documented as to their purpose and
> derivation.
> > The conventional way to do this is via a constant with appropriate
> Javadoc.
> >
> > In other words, why is the suffix ".activated" ?
> > Why not ".alive" or ".on" or ".randomString"?
> >
> >> JLouis
> >>
> >>
> >> 2013/10/3 sebb <sebbaz@gmail.com>
> >>
> >>> On 2 October 2013 21:12,  <jlmonteiro@apache.org> wrote:
> >>> > Author: jlmonteiro
> >>> > Date: Wed Oct  2 20:12:29 2013
> >>> > New Revision: 1528612
> >>> >
> >>> > URL: http://svn.apache.org/r1528612
> >>> > Log:
> >>> > Fixing configuration property typo
> >>> >
> >>> > Modified:
> >>> >
> >>>
> commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
> >>> >
> >>> > Modified:
> >>>
> commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
> >>> > URL:
> >>>
> http://svn.apache.org/viewvc/commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java?rev=1528612&r1=1528611&r2=1528612&view=diff
> >>> >
> >>>
> ==============================================================================
> >>> > ---
> >>>
> commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
> >>> (original)
> >>> > +++
> >>>
> commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
> >>> Wed Oct  2 20:12:29 2013
> >>> > @@ -35,7 +35,7 @@ public final class PluginRepository {
> >>> >              if (name == null) {
> >>> >                  throw new IllegalArgumentException("plugin name
> can't
> >>> be null");
> >>> >              }
> >>> > -            if (!Configuration.is(name + "activated", true)) {
> >>> > +            if (!Configuration.is(name + ".activated", true)) {
> >>>
> >>> I assume that this string is used elsewhere within monitoring?
> >>> If so, it should be defined once as a String constant (with Javadoc)
> >>> and used throughout.
> >>> Or there could be a method to convert a name by appending the suffix.
> >>>
> >>> >                  continue;
> >>> >              }
> >>> >
> >>> >
> >>> >
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >>> For additional commands, e-mail: dev-help@commons.apache.org
> >>>
> >>>
> >>
> >>
> >> --
> >> Jean-Louis
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message