commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Carman <ja...@carmanconsulting.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:11:32 GMT
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
View raw message