You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2013/10/03 01:45:11 UTC
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
On 2 October 2013 21:12, <jl...@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
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by sebb <se...@gmail.com>.
On 5 October 2013 10:51, Romain Manni-Bucau <rm...@gmail.com> wrote:
> Activated cause it answer the question (if) "is activated"
But why not ".active" or ".isActive"?
There must be some other bit of code that sets the property initially.
It may just be a convention that the user has to set the property.
Whatever the reason for choosing this particular string, it needs to
be documented at the point of use (possibly via a link to existing
docs).
> Le 5 oct. 2013 00:46, "sebb" <se...@gmail.com> a écrit :
>
>> On 4 October 2013 23:23, James Carman <ja...@carmanconsulting.com> wrote:
>> > On Fri, Oct 4, 2013 at 6:11 PM, sebb <se...@gmail.com> wrote:
>> >>
>> >> Back to the case in point: why is the string ".activate" and not
>> anything else?
>> >>
>> >
>> > Why not just extract a constant and name it something logical like
>> > "ACTIVATED_INDICATOR"? Then, the actual value of the constant can be
>> > whatever you want. The value of the string isn't important (other
>> > than making it a bit more human-readable of course). What is
>> > important is that you use the string in a consistent fashion
>> > throughout your code. By not using a constant, it makes it more
>> > likely to fat-finger the value when you type it again somewhere.
>> > Anyway, I'm sure I'm not telling you anything new. I just don't think
>> > it's relevant to go into a lengthy discussion about the string that's
>> > used as some sort of marker. Just extract the constant (and use it)
>> > and move on.
>>
>> That was exactly my point.
>> However, I don't think the value is arbitrary, given that it is only used
>> once.
>> It looks to me as though the code is relying on some external
>> convention for property naming.
>>
>> > ---------------------------------------------------------------------
>> > 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
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Activated cause it answer the question (if) "is activated"
Le 5 oct. 2013 00:46, "sebb" <se...@gmail.com> a écrit :
> On 4 October 2013 23:23, James Carman <ja...@carmanconsulting.com> wrote:
> > On Fri, Oct 4, 2013 at 6:11 PM, sebb <se...@gmail.com> wrote:
> >>
> >> Back to the case in point: why is the string ".activate" and not
> anything else?
> >>
> >
> > Why not just extract a constant and name it something logical like
> > "ACTIVATED_INDICATOR"? Then, the actual value of the constant can be
> > whatever you want. The value of the string isn't important (other
> > than making it a bit more human-readable of course). What is
> > important is that you use the string in a consistent fashion
> > throughout your code. By not using a constant, it makes it more
> > likely to fat-finger the value when you type it again somewhere.
> > Anyway, I'm sure I'm not telling you anything new. I just don't think
> > it's relevant to go into a lengthy discussion about the string that's
> > used as some sort of marker. Just extract the constant (and use it)
> > and move on.
>
> That was exactly my point.
> However, I don't think the value is arbitrary, given that it is only used
> once.
> It looks to me as though the code is relying on some external
> convention for property naming.
>
> > ---------------------------------------------------------------------
> > 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
>
>
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by sebb <se...@gmail.com>.
On 4 October 2013 23:23, James Carman <ja...@carmanconsulting.com> wrote:
> On Fri, Oct 4, 2013 at 6:11 PM, sebb <se...@gmail.com> wrote:
>>
>> Back to the case in point: why is the string ".activate" and not anything else?
>>
>
> Why not just extract a constant and name it something logical like
> "ACTIVATED_INDICATOR"? Then, the actual value of the constant can be
> whatever you want. The value of the string isn't important (other
> than making it a bit more human-readable of course). What is
> important is that you use the string in a consistent fashion
> throughout your code. By not using a constant, it makes it more
> likely to fat-finger the value when you type it again somewhere.
> Anyway, I'm sure I'm not telling you anything new. I just don't think
> it's relevant to go into a lengthy discussion about the string that's
> used as some sort of marker. Just extract the constant (and use it)
> and move on.
That was exactly my point.
However, I don't think the value is arbitrary, given that it is only used once.
It looks to me as though the code is relying on some external
convention for property naming.
> ---------------------------------------------------------------------
> 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
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Then so will say no need to keep holding memory for this startup flag :p
Monitoring will just go ahead and will surely be reworked a bit so no need
to stop on details now JL
Le 6 oct. 2013 21:49, "Jean-Louis MONTEIRO" <je...@gmail.com> a écrit :
> Hey guys,
>
> I don't want to interrupt such an interesting discussion.
> I just wanted to fix a typo I caught up.
>
> If creating a constant with the right javadoc is ok, let's just do it,
> there is no big issue on that.
> Lemme me know if the following works.
>
> jlmonteiro$ svn ci -m "Adding a constant for the activation flag."
>
> reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
> Sending
>
> reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
> Transmitting file data .
> Committed revision 1529670.
>
> JLouis
>
>
> 2013/10/5 Christian Grobmeier <gr...@gmail.com>
>
> > On 5 Oct 2013, at 14:29, James Carman wrote:
> >
> > On Sat, Oct 5, 2013 at 7:03 AM, Benedikt Ritter <be...@gmail.com>
> >> wrote:
> >>
> >>>
> >>> I'm not sure I agree with all of your points. Yes, the sandbox is a
> >>> place to try new ideas out. Does this mean certain quality criterions
> do
> >>> not apply? I don't think so. This all has to be corrected before
> promotion,
> >>> so why not make it correct right from the start?
> >>>
> >>>
> >> Sometimes it helps people to get their ideas down and working without
> >> worrying about "correctness." That's why writers have a rough draft,
> >> after all. The creative process is best left unencumbered when
> >> nurturing a new idea. The sandbox is all about letting folks work on
> >> an idea they have. It's a *sandbox*! Yes, it does mean that certain
> >> quality criteria do not apply, until the point when the component
> >> wishes to graduate to "proper." At that point, we can put on our
> >> white gloves and go over every last line (or look at Sonar reports) if
> >> we wish.
> >>
> >> Is pointing out that something may be improved nit-picking? Well, I
> >>> think it depends :-) Just sending a -1 for a commit like this would
> >>> definitely be. In this case an improvement has been pointed out. I'm
> more
> >>> then happy for feedback like this, because it helps me become a better
> >>> developer. And in the end, discussing commits is part of the game ;-)
> >>>
> >>>
> >> Yes, in a sandbox environment, pointing out a small "magic string"
> >> infraction is nit-picking. Leave the authors alone and let them work
> >> through their ideas. If you want to help out with the code, jump in
> >> and help. It takes longer to write an email and participate in the
> >> back-and-forth that ensues than it does to just fix it yourself. For
> >> issues like this, we really need to be using a tool like Sonar. Sonar
> >> will objectively look at the code for infractions such as this (among
> >> many others). The author can then look at the Sonar reports and see
> >> areas that might need improvement at their leisure (the group will
> >> also do so before graduating the component to proper). The other
> >> great thing about Sonar is that it has verbiage in there about why the
> >> rule is a rule and what can be done to fix the issue, so it's also a
> >> teaching tool. Most likely, the author fully understands why it's bad
> >> to have "magic strings" in their code, but just wanted to get their
> >> ideas into code and working before worrying about such issues. They
> >> can clean it up later (or some of us can jump in and help).
> >>
> >> This is the exact reason that I personally would *never* start a new
> >> project here at Commons again. I would invite certain colleagues to
> >> collaborate on github or something and then later bring the code into
> >> the organization.
> >>
> >
> > I am very sorry to say that I feel pretty similar.
> >
> > Commons is a lot on nit-picking. It once was an innovating place.
> > But often we discuss to many formalities or jdk 1.3 vs 1.5 vs 1.7 instead
> > of just making releases. Working at Commons is pretty much complicated.
> > It starts with a super complicated black magic parent pom and ends with
> > discussing
> > the value magic strings in the sandbox.
> >
> > I see your point Dominik. We need to discuss commits. But not at any
> time,
> > often not in the sandbox and not at any place.
> >
> > We are slow. Guava isn't slow. That's why more and more people walk over
> > to that place.
> > The way to long discussion on using annotation in Commons Collections is
> a
> > good example.
> >
> > Just want to say, the topic has changed. If anybody has the energy to
> > change the subject, it's the right time.
> >
> >
> > ------------------------------**------------------------------**---------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.**apache.org<
> dev-unsubscribe@commons.apache.org>
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> --
> Jean-Louis
>
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Jean-Louis MONTEIRO <je...@gmail.com>.
Hey guys,
I don't want to interrupt such an interesting discussion.
I just wanted to fix a typo I caught up.
If creating a constant with the right javadoc is ok, let's just do it,
there is no big issue on that.
Lemme me know if the following works.
jlmonteiro$ svn ci -m "Adding a constant for the activation flag."
reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Sending
reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Transmitting file data .
Committed revision 1529670.
JLouis
2013/10/5 Christian Grobmeier <gr...@gmail.com>
> On 5 Oct 2013, at 14:29, James Carman wrote:
>
> On Sat, Oct 5, 2013 at 7:03 AM, Benedikt Ritter <be...@gmail.com>
>> wrote:
>>
>>>
>>> I'm not sure I agree with all of your points. Yes, the sandbox is a
>>> place to try new ideas out. Does this mean certain quality criterions do
>>> not apply? I don't think so. This all has to be corrected before promotion,
>>> so why not make it correct right from the start?
>>>
>>>
>> Sometimes it helps people to get their ideas down and working without
>> worrying about "correctness." That's why writers have a rough draft,
>> after all. The creative process is best left unencumbered when
>> nurturing a new idea. The sandbox is all about letting folks work on
>> an idea they have. It's a *sandbox*! Yes, it does mean that certain
>> quality criteria do not apply, until the point when the component
>> wishes to graduate to "proper." At that point, we can put on our
>> white gloves and go over every last line (or look at Sonar reports) if
>> we wish.
>>
>> Is pointing out that something may be improved nit-picking? Well, I
>>> think it depends :-) Just sending a -1 for a commit like this would
>>> definitely be. In this case an improvement has been pointed out. I'm more
>>> then happy for feedback like this, because it helps me become a better
>>> developer. And in the end, discussing commits is part of the game ;-)
>>>
>>>
>> Yes, in a sandbox environment, pointing out a small "magic string"
>> infraction is nit-picking. Leave the authors alone and let them work
>> through their ideas. If you want to help out with the code, jump in
>> and help. It takes longer to write an email and participate in the
>> back-and-forth that ensues than it does to just fix it yourself. For
>> issues like this, we really need to be using a tool like Sonar. Sonar
>> will objectively look at the code for infractions such as this (among
>> many others). The author can then look at the Sonar reports and see
>> areas that might need improvement at their leisure (the group will
>> also do so before graduating the component to proper). The other
>> great thing about Sonar is that it has verbiage in there about why the
>> rule is a rule and what can be done to fix the issue, so it's also a
>> teaching tool. Most likely, the author fully understands why it's bad
>> to have "magic strings" in their code, but just wanted to get their
>> ideas into code and working before worrying about such issues. They
>> can clean it up later (or some of us can jump in and help).
>>
>> This is the exact reason that I personally would *never* start a new
>> project here at Commons again. I would invite certain colleagues to
>> collaborate on github or something and then later bring the code into
>> the organization.
>>
>
> I am very sorry to say that I feel pretty similar.
>
> Commons is a lot on nit-picking. It once was an innovating place.
> But often we discuss to many formalities or jdk 1.3 vs 1.5 vs 1.7 instead
> of just making releases. Working at Commons is pretty much complicated.
> It starts with a super complicated black magic parent pom and ends with
> discussing
> the value magic strings in the sandbox.
>
> I see your point Dominik. We need to discuss commits. But not at any time,
> often not in the sandbox and not at any place.
>
> We are slow. Guava isn't slow. That's why more and more people walk over
> to that place.
> The way to long discussion on using annotation in Commons Collections is a
> good example.
>
> Just want to say, the topic has changed. If anybody has the energy to
> change the subject, it's the right time.
>
>
> ------------------------------**------------------------------**---------
> To unsubscribe, e-mail: dev-unsubscribe@commons.**apache.org<de...@commons.apache.org>
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
Jean-Louis
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Olivier Lamy <ol...@apache.org>.
On 6 October 2013 07:22, Christian Grobmeier <gr...@gmail.com> wrote:
> On 5 Oct 2013, at 14:29, James Carman wrote:
>
>> On Sat, Oct 5, 2013 at 7:03 AM, Benedikt Ritter <be...@gmail.com>
>> wrote:
>>>
>>>
>>> I'm not sure I agree with all of your points. Yes, the sandbox is a place
>>> to try new ideas out. Does this mean certain quality criterions do not
>>> apply? I don't think so. This all has to be corrected before promotion, so
>>> why not make it correct right from the start?
>>>
>>
>> Sometimes it helps people to get their ideas down and working without
>> worrying about "correctness." That's why writers have a rough draft,
>> after all. The creative process is best left unencumbered when
>> nurturing a new idea. The sandbox is all about letting folks work on
>> an idea they have. It's a *sandbox*! Yes, it does mean that certain
>> quality criteria do not apply, until the point when the component
>> wishes to graduate to "proper." At that point, we can put on our
>> white gloves and go over every last line (or look at Sonar reports) if
>> we wish.
>>
>>> Is pointing out that something may be improved nit-picking? Well, I think
>>> it depends :-) Just sending a -1 for a commit like this would definitely be.
>>> In this case an improvement has been pointed out. I'm more then happy for
>>> feedback like this, because it helps me become a better developer. And in
>>> the end, discussing commits is part of the game ;-)
>>>
>>
>> Yes, in a sandbox environment, pointing out a small "magic string"
>> infraction is nit-picking. Leave the authors alone and let them work
>> through their ideas. If you want to help out with the code, jump in
>> and help. It takes longer to write an email and participate in the
>> back-and-forth that ensues than it does to just fix it yourself. For
>> issues like this, we really need to be using a tool like Sonar. Sonar
>> will objectively look at the code for infractions such as this (among
>> many others). The author can then look at the Sonar reports and see
>> areas that might need improvement at their leisure (the group will
>> also do so before graduating the component to proper). The other
>> great thing about Sonar is that it has verbiage in there about why the
>> rule is a rule and what can be done to fix the issue, so it's also a
>> teaching tool. Most likely, the author fully understands why it's bad
>> to have "magic strings" in their code, but just wanted to get their
>> ideas into code and working before worrying about such issues. They
>> can clean it up later (or some of us can jump in and help).
>>
>> This is the exact reason that I personally would *never* start a new
>> project here at Commons again. I would invite certain colleagues to
>> collaborate on github or something and then later bring the code into
>> the organization.
>
>
> I am very sorry to say that I feel pretty similar.
>
> Commons is a lot on nit-picking. It once was an innovating place.
> But often we discuss to many formalities or jdk 1.3 vs 1.5 vs 1.7 instead
> of just making releases. Working at Commons is pretty much complicated.
> It starts with a super complicated black magic parent pom and ends with
> discussing
> the value magic strings in the sandbox.
>
> I see your point Dominik. We need to discuss commits. But not at any time,
> often not in the sandbox and not at any place.
>
> We are slow. Guava isn't slow. That's why more and more people walk over to
> that place.
> The way to long discussion on using annotation in Commons Collections is a
> good example.
I cannot agree more....
>
> Just want to say, the topic has changed. If anybody has the energy to change
> the subject, it's the right time.
>
Will need to much energy and too many emails to write.
Most of the time all of those procedural minor stuff just kill motivation...
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
--
Olivier Lamy
Ecetera: http://ecetera.com.au
http://twitter.com/olamy | http://linkedin.com/in/olamy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1528612 -
/commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Christian Grobmeier <gr...@gmail.com>.
On 5 Oct 2013, at 14:29, James Carman wrote:
> On Sat, Oct 5, 2013 at 7:03 AM, Benedikt Ritter <be...@gmail.com>
> wrote:
>>
>> I'm not sure I agree with all of your points. Yes, the sandbox is a
>> place to try new ideas out. Does this mean certain quality criterions
>> do not apply? I don't think so. This all has to be corrected before
>> promotion, so why not make it correct right from the start?
>>
>
> Sometimes it helps people to get their ideas down and working without
> worrying about "correctness." That's why writers have a rough draft,
> after all. The creative process is best left unencumbered when
> nurturing a new idea. The sandbox is all about letting folks work on
> an idea they have. It's a *sandbox*! Yes, it does mean that certain
> quality criteria do not apply, until the point when the component
> wishes to graduate to "proper." At that point, we can put on our
> white gloves and go over every last line (or look at Sonar reports) if
> we wish.
>
>> Is pointing out that something may be improved nit-picking? Well, I
>> think it depends :-) Just sending a -1 for a commit like this would
>> definitely be. In this case an improvement has been pointed out. I'm
>> more then happy for feedback like this, because it helps me become a
>> better developer. And in the end, discussing commits is part of the
>> game ;-)
>>
>
> Yes, in a sandbox environment, pointing out a small "magic string"
> infraction is nit-picking. Leave the authors alone and let them work
> through their ideas. If you want to help out with the code, jump in
> and help. It takes longer to write an email and participate in the
> back-and-forth that ensues than it does to just fix it yourself. For
> issues like this, we really need to be using a tool like Sonar. Sonar
> will objectively look at the code for infractions such as this (among
> many others). The author can then look at the Sonar reports and see
> areas that might need improvement at their leisure (the group will
> also do so before graduating the component to proper). The other
> great thing about Sonar is that it has verbiage in there about why the
> rule is a rule and what can be done to fix the issue, so it's also a
> teaching tool. Most likely, the author fully understands why it's bad
> to have "magic strings" in their code, but just wanted to get their
> ideas into code and working before worrying about such issues. They
> can clean it up later (or some of us can jump in and help).
>
> This is the exact reason that I personally would *never* start a new
> project here at Commons again. I would invite certain colleagues to
> collaborate on github or something and then later bring the code into
> the organization.
I am very sorry to say that I feel pretty similar.
Commons is a lot on nit-picking. It once was an innovating place.
But often we discuss to many formalities or jdk 1.3 vs 1.5 vs 1.7
instead
of just making releases. Working at Commons is pretty much complicated.
It starts with a super complicated black magic parent pom and ends with
discussing
the value magic strings in the sandbox.
I see your point Dominik. We need to discuss commits. But not at any
time,
often not in the sandbox and not at any place.
We are slow. Guava isn't slow. That's why more and more people walk over
to that place.
The way to long discussion on using annotation in Commons Collections is
a good example.
Just want to say, the topic has changed. If anybody has the energy to
change the subject, it's the right time.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Fyi doc is here
http://commons.apache.org/sandbox/commons-monitoring/configuration.html
Le 5 oct. 2013 14:30, "James Carman" <jc...@carmanconsulting.com> a
écrit :
> On Sat, Oct 5, 2013 at 7:03 AM, Benedikt Ritter <be...@gmail.com>
> wrote:
> >
> > I'm not sure I agree with all of your points. Yes, the sandbox is a
> place to try new ideas out. Does this mean certain quality criterions do
> not apply? I don't think so. This all has to be corrected before promotion,
> so why not make it correct right from the start?
> >
>
> Sometimes it helps people to get their ideas down and working without
> worrying about "correctness." That's why writers have a rough draft,
> after all. The creative process is best left unencumbered when
> nurturing a new idea. The sandbox is all about letting folks work on
> an idea they have. It's a *sandbox*! Yes, it does mean that certain
> quality criteria do not apply, until the point when the component
> wishes to graduate to "proper." At that point, we can put on our
> white gloves and go over every last line (or look at Sonar reports) if
> we wish.
>
> > Is pointing out that something may be improved nit-picking? Well, I
> think it depends :-) Just sending a -1 for a commit like this would
> definitely be. In this case an improvement has been pointed out. I'm more
> then happy for feedback like this, because it helps me become a better
> developer. And in the end, discussing commits is part of the game ;-)
> >
>
> Yes, in a sandbox environment, pointing out a small "magic string"
> infraction is nit-picking. Leave the authors alone and let them work
> through their ideas. If you want to help out with the code, jump in
> and help. It takes longer to write an email and participate in the
> back-and-forth that ensues than it does to just fix it yourself. For
> issues like this, we really need to be using a tool like Sonar. Sonar
> will objectively look at the code for infractions such as this (among
> many others). The author can then look at the Sonar reports and see
> areas that might need improvement at their leisure (the group will
> also do so before graduating the component to proper). The other
> great thing about Sonar is that it has verbiage in there about why the
> rule is a rule and what can be done to fix the issue, so it's also a
> teaching tool. Most likely, the author fully understands why it's bad
> to have "magic strings" in their code, but just wanted to get their
> ideas into code and working before worrying about such issues. They
> can clean it up later (or some of us can jump in and help).
>
> This is the exact reason that I personally would *never* start a new
> project here at Commons again. I would invite certain colleagues to
> collaborate on github or something and then later bring the code into
> the organization.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by James Carman <jc...@carmanconsulting.com>.
On Sat, Oct 5, 2013 at 7:03 AM, Benedikt Ritter <be...@gmail.com> wrote:
>
> I'm not sure I agree with all of your points. Yes, the sandbox is a place to try new ideas out. Does this mean certain quality criterions do not apply? I don't think so. This all has to be corrected before promotion, so why not make it correct right from the start?
>
Sometimes it helps people to get their ideas down and working without
worrying about "correctness." That's why writers have a rough draft,
after all. The creative process is best left unencumbered when
nurturing a new idea. The sandbox is all about letting folks work on
an idea they have. It's a *sandbox*! Yes, it does mean that certain
quality criteria do not apply, until the point when the component
wishes to graduate to "proper." At that point, we can put on our
white gloves and go over every last line (or look at Sonar reports) if
we wish.
> Is pointing out that something may be improved nit-picking? Well, I think it depends :-) Just sending a -1 for a commit like this would definitely be. In this case an improvement has been pointed out. I'm more then happy for feedback like this, because it helps me become a better developer. And in the end, discussing commits is part of the game ;-)
>
Yes, in a sandbox environment, pointing out a small "magic string"
infraction is nit-picking. Leave the authors alone and let them work
through their ideas. If you want to help out with the code, jump in
and help. It takes longer to write an email and participate in the
back-and-forth that ensues than it does to just fix it yourself. For
issues like this, we really need to be using a tool like Sonar. Sonar
will objectively look at the code for infractions such as this (among
many others). The author can then look at the Sonar reports and see
areas that might need improvement at their leisure (the group will
also do so before graduating the component to proper). The other
great thing about Sonar is that it has verbiage in there about why the
rule is a rule and what can be done to fix the issue, so it's also a
teaching tool. Most likely, the author fully understands why it's bad
to have "magic strings" in their code, but just wanted to get their
ideas into code and working before worrying about such issues. They
can clean it up later (or some of us can jump in and help).
This is the exact reason that I personally would *never* start a new
project here at Commons again. I would invite certain colleagues to
collaborate on github or something and then later bring the code into
the organization.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Benedikt Ritter <be...@gmail.com>.
Hi James,
Send from my mobile device
> Am 05.10.2013 um 00:33 schrieb James Carman <ja...@carmanconsulting.com>:
>
> Also, since when did we start nit-picking code in the sandbox? Why
> not leave these folks alone and let them work out their ideas? The
> sandbox should be an area where folks can play around with stuff and
> share new ideas without being hassled by folks. If we make it a pain
> in the butt, then people will just go elsewhere with their cool ideas.
> It's one thing if you're collaborating on the project, but trolling
> commit logs and picking out things you don't like is just plain rude,
> IMHO.
I'm not sure I agree with all of your points. Yes, the sandbox is a place to try new ideas out. Does this mean certain quality criterions do not apply? I don't think so. This all has to be corrected before promotion, so why not make it correct right from the start?
Is pointing out that something may be improved nit-picking? Well, I think it depends :-) Just sending a -1 for a commit like this would definitely be. In this case an improvement has been pointed out. I'm more then happy for feedback like this, because it helps me become a better developer. And in the end, discussing commits is part of the game ;-)
Regards,
Benedikt
>
>> On Fri, Oct 4, 2013 at 6:23 PM, James Carman <ja...@carmanconsulting.com> wrote:
>>> On Fri, Oct 4, 2013 at 6:11 PM, sebb <se...@gmail.com> wrote:
>>>
>>> Back to the case in point: why is the string ".activate" and not anything else?
>>
>> Why not just extract a constant and name it something logical like
>> "ACTIVATED_INDICATOR"? Then, the actual value of the constant can be
>> whatever you want. The value of the string isn't important (other
>> than making it a bit more human-readable of course). What is
>> important is that you use the string in a consistent fashion
>> throughout your code. By not using a constant, it makes it more
>> likely to fat-finger the value when you type it again somewhere.
>> Anyway, I'm sure I'm not telling you anything new. I just don't think
>> it's relevant to go into a lengthy discussion about the string that's
>> used as some sort of marker. Just extract the constant (and use it)
>> and move on.
>
> ---------------------------------------------------------------------
> 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
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by James Carman <ja...@carmanconsulting.com>.
Also, since when did we start nit-picking code in the sandbox? Why
not leave these folks alone and let them work out their ideas? The
sandbox should be an area where folks can play around with stuff and
share new ideas without being hassled by folks. If we make it a pain
in the butt, then people will just go elsewhere with their cool ideas.
It's one thing if you're collaborating on the project, but trolling
commit logs and picking out things you don't like is just plain rude,
IMHO.
On Fri, Oct 4, 2013 at 6:23 PM, James Carman <ja...@carmanconsulting.com> wrote:
> On Fri, Oct 4, 2013 at 6:11 PM, sebb <se...@gmail.com> wrote:
>>
>> Back to the case in point: why is the string ".activate" and not anything else?
>>
>
> Why not just extract a constant and name it something logical like
> "ACTIVATED_INDICATOR"? Then, the actual value of the constant can be
> whatever you want. The value of the string isn't important (other
> than making it a bit more human-readable of course). What is
> important is that you use the string in a consistent fashion
> throughout your code. By not using a constant, it makes it more
> likely to fat-finger the value when you type it again somewhere.
> Anyway, I'm sure I'm not telling you anything new. I just don't think
> it's relevant to go into a lengthy discussion about the string that's
> used as some sort of marker. Just extract the constant (and use it)
> and move on.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Fri, Oct 4, 2013 at 6:11 PM, sebb <se...@gmail.com> wrote:
>
> Back to the case in point: why is the string ".activate" and not anything else?
>
Why not just extract a constant and name it something logical like
"ACTIVATED_INDICATOR"? Then, the actual value of the constant can be
whatever you want. The value of the string isn't important (other
than making it a bit more human-readable of course). What is
important is that you use the string in a consistent fashion
throughout your code. By not using a constant, it makes it more
likely to fat-finger the value when you type it again somewhere.
Anyway, I'm sure I'm not telling you anything new. I just don't think
it's relevant to go into a lengthy discussion about the string that's
used as some sort of marker. Just extract the constant (and use it)
and move on.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by sebb <se...@gmail.com>.
On 4 October 2013 21:09, Romain Manni-Bucau <rm...@gmail.com> wrote:
> Hi
>
> It should be done in the doc iirc. Having a constant would just mean
> loosing a line with no benefit excepted needing to browse code instead of
> reading it inline in this case cause the prop will not be used in the code
> at all so id stay it like it...moreover thats really a detail for the
> product imo
The advantage of using a field is that it can have Javadoc.
Also there are code validation tools that can detect 'magic' constants
when used inline; they expect constants to be defined as such.
Most IDEs these days will show the value of the constant when hovering over it.
What's important is that people reading the code see the explanation
right by where the string is used.
Back to the case in point: why is the string ".activate" and not anything else?
> Le 4 oct. 2013 20:29, "sebb" <se...@gmail.com> a écrit :
>
>> On 4 October 2013 18:04, Jean-Louis MONTEIRO <je...@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 <se...@gmail.com>
>> >
>> >> On 2 October 2013 21:12, <jl...@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
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Hi
It should be done in the doc iirc. Having a constant would just mean
loosing a line with no benefit excepted needing to browse code instead of
reading it inline in this case cause the prop will not be used in the code
at all so id stay it like it...moreover thats really a detail for the
product imo
Le 4 oct. 2013 20:29, "sebb" <se...@gmail.com> a écrit :
> On 4 October 2013 18:04, Jean-Louis MONTEIRO <je...@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 <se...@gmail.com>
> >
> >> On 2 October 2013 21:12, <jl...@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
>
>
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by James Carman <ja...@carmanconsulting.com>.
I meant conceptually. I see no reason we wouldn't let sandbox
projects use Sonar also. It would help us understand how ready the
code is for "prime time" before we promote it to "proper." I was
trying to find a link to our Sonar installation, but my google-fu is
failing me at the moment.
On Fri, Oct 4, 2013 at 5:51 PM, Romain Manni-Bucau
<rm...@gmail.com> wrote:
> I thought sandbox projects were not included, any link on it?
> Le 4 oct. 2013 23:02, "James Carman" <ja...@carmanconsulting.com> a écrit :
>
>> Sure, why not?
>>
>> On Fri, Oct 4, 2013 at 4:58 PM, Romain Manni-Bucau
>> <rm...@gmail.com> wrote:
>> > Even for sandbox?
>> > Le 4 oct. 2013 22:12, "James Carman" <ja...@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 <se...@gmail.com> wrote:
>> >> > On 4 October 2013 18:04, Jean-Louis MONTEIRO <je...@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 <se...@gmail.com>
>> >> >>
>> >> >>> On 2 October 2013 21:12, <jl...@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
>> >>
>> >>
>>
>> ---------------------------------------------------------------------
>> 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
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
I thought sandbox projects were not included, any link on it?
Le 4 oct. 2013 23:02, "James Carman" <ja...@carmanconsulting.com> a écrit :
> Sure, why not?
>
> On Fri, Oct 4, 2013 at 4:58 PM, Romain Manni-Bucau
> <rm...@gmail.com> wrote:
> > Even for sandbox?
> > Le 4 oct. 2013 22:12, "James Carman" <ja...@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 <se...@gmail.com> wrote:
> >> > On 4 October 2013 18:04, Jean-Louis MONTEIRO <je...@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 <se...@gmail.com>
> >> >>
> >> >>> On 2 October 2013 21:12, <jl...@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
> >>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by James Carman <ja...@carmanconsulting.com>.
Sure, why not?
On Fri, Oct 4, 2013 at 4:58 PM, Romain Manni-Bucau
<rm...@gmail.com> wrote:
> Even for sandbox?
> Le 4 oct. 2013 22:12, "James Carman" <ja...@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 <se...@gmail.com> wrote:
>> > On 4 October 2013 18:04, Jean-Louis MONTEIRO <je...@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 <se...@gmail.com>
>> >>
>> >>> On 2 October 2013 21:12, <jl...@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
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Romain Manni-Bucau <rm...@gmail.com>.
Even for sandbox?
Le 4 oct. 2013 22:12, "James Carman" <ja...@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 <se...@gmail.com> wrote:
> > On 4 October 2013 18:04, Jean-Louis MONTEIRO <je...@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 <se...@gmail.com>
> >>
> >>> On 2 October 2013 21:12, <jl...@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
>
>
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by James Carman <ja...@carmanconsulting.com>.
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 <se...@gmail.com> wrote:
> On 4 October 2013 18:04, Jean-Louis MONTEIRO <je...@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 <se...@gmail.com>
>>
>>> On 2 October 2013 21:12, <jl...@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
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by sebb <se...@gmail.com>.
On 4 October 2013 18:04, Jean-Louis MONTEIRO <je...@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 <se...@gmail.com>
>
>> On 2 October 2013 21:12, <jl...@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
Re: svn commit: r1528612 - /commons/sandbox/monitoring/trunk/reporting/src/main/java/org/apache/commons/monitoring/reporting/web/plugin/PluginRepository.java
Posted by Jean-Louis MONTEIRO <je...@gmail.com>.
Apologize for the late answer.
Not sure to understand the purpose of the request.
Could you detail cause it's not used anywhere else?
JLouis
2013/10/3 sebb <se...@gmail.com>
> On 2 October 2013 21:12, <jl...@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