Re: [java-gnome-hackers] added getInverted and setInverted to Range

[Peter M. <Pet...@gm...>]

Hi Andrew,
Thanks for your comments.

I have now reworked my patch to incorporate your comments. I have 
attached the bundle to my mail.

I hope it is now ready to be merged.
Regards Peter

Andrew Cowie wrote:
> On Sat, 2009-05-09 at 12:00 +0200, Peter Mosseveld wrote:
>> I have attached a bundle containing these methods.
> 
> Hey, lovely.
> 
>>  This is my first 
>> patch. So I hope I have done everything right.
> 
> Just some minor details that come under "style" and "consistency"
> 
>> -     * Retreive the value currently indicated by this Range instance.
>> +     * Retrieve the value currently indicated by this Range instance.
> 
> Thanks for fixing the spelling :)
> 
>> +     * Retrieve the current value set by setInverted
> 
> When you need to refer to another method, use a @link tag. In this case,
> 
>         ... yada blah yada {@link #setInverted(boolean) setInverted()}
>         yada yada blah blah...
> 
> 
> As a very high level thing, rather than saying "Get the value" try
> something more specific, perhaps "Get whether or not the rate scale of
> this Range is inverted; see {@link ...}."
> 
> [I have often skipped bothering to write a getter simply because, well,
> you wrote the code, and you know full well whether you changed it from
> the default or not. But that's just me; nothing wrong with exposing a
> getter if you need it and/or are willing to put the work in to offer it]
> 
>> +    /**
>> +     * Inverts the way the value changes when moving the slider
>> +     *
>> +     * Ranges normally move from...
> 
> You need a <p> tag in there if you are trying to start a new paragraph.
> Otherwise the whitespace is swallowed (as you'd expect with HTML as the
> end result).
> 
>> +     * ...higher values at the top or on the right rather than on the
>> +     * bottom or left.
>> +     */
> 
> Please add a @since tag. Assume your patch will be accepted and in the
> next release :) For example, the latest release was 4.0.11; 'mainline'
> says it is 4.0.12-dev right now; assume you'll be in 4.0.12 and put
> 
>         * @since 4.0.12
> 
> This has the wonderful effect of saving the maintainer the trouble of
> adding it for you, and anything that saves the maintainer trouble is
> well regarded :).
> 
> So maybe just touch up those things and then we can merge it!
> 
> AfC
> Sydney
> 
> P.S. I find it a bit easier if you include a bundle as an attachment
> rather than inline, but that's just me.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> ------------------------------------------------------------------------------
> The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your
> production scanning environment may not be a perfect world - but thanks to
> Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700
> Series Scanner you'll get full speed at 300 dpi even with all image 
> processing features enabled. http://p.sf.net/sfu/kodak-com
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> java-gnome-hackers mailing list
> jav...@li...
> https://lists.sourceforge.net/lists/listinfo/java-gnome-hackers