Thread: [java-gnome-hackers] ProgressBar Orientation patch (Need revision)
Brought to you by:
afcowie
From: Goundy <go...@gm...> - 2009-01-11 19:38:45
Attachments:
ProgressBar-Orientation.patch
progress.java
|
Hi dear hackers, Well, I just added the ProgressBarOrientation support and tried to document it but my english skills are too limited. So I wish, if someone has a few time and would like to take look at my patch, that'd be nice :) Attached the patch + a java class as example of use. Cheers |
From: Andrew C. <an...@op...> - 2009-01-13 02:04:25
|
In the main, your patch is fine. Some QA-ish points: On Sun, 2009-01-11 at 20:37 +0100, Miloud Bel wrote: > - * Copyright (c) 2007-2008 Operational Dynamics Consulting Pty Ltd, > and Others > + * Copyright (c) 2009 Operational Dynamics Consulting Pty Ltd, and > Others The correct change is to make it "2007-2009". > + /** > + * Set the orientation of the progress bar > + * Four possible values: > + * > + * LEFT_TO_RIGHT > + * RIGHT_TO_LEFT > + * TOP_TO_BOTTOM > + * BOTTOM_TO_TOP If you run javadoc over this (ie, do `make doc`) you will discover that whitespace is not preserved (it is, to all intents, HTML) and so you'll need to make that an <ul> list or use <br> if you want to have four items. I'm not really sure you need to list them, but whatever. http://java-gnome.sourceforge.net/4.0/doc/api/org/gnome/pango/Style.html is an example of where we were verbose about it. You really ought to preview in a web browser. Like I said, $ make doc > + * @param orientation > + * a ProgressBarOrientation value You don't need that. > + /** > + * Get the current progress bar orientation if you're going to talk about the proper name of a class, then use the class name: * Get orientation currently in effect in this ProgressBar. or whatever. Perhaps Kenneth will volunteer to help you with the prose. ++ You also really need to run the code formatter before committing. Serkan can help you with that, but if you're not in Eclipse: $ make format and if you are, Source -> Format. AfC Sydney -- Andrew Frederick Cowie Operational Dynamics is an operations and engineering consultancy focusing on IT strategy, organizational architecture, systems review, and effective procedures for change management: enabling successful deployment of mission critical information technology in enterprises, worldwide. http://www.operationaldynamics.com/ Sydney New York Toronto London |
From: Goundy <go...@gm...> - 2009-01-17 15:16:56
|
Hi. Thank you Afc for your hints I corrected all what you pointed to. And sorry for the time I put to do it but recently I got really hard exams but fortunately I succeeded. (Two more very easy exams and I'm done :-)) Well I hope this patch is better than the first one. If there's more changes I should do, just reply back :-) Thanks and have a good day Andrew Cowie wrote: > In the main, your patch is fine. Some QA-ish points: > > On Sun, 2009-01-11 at 20:37 +0100, Miloud Bel wrote: > >> - * Copyright (c) 2007-2008 Operational Dynamics Consulting Pty Ltd, >> and Others >> + * Copyright (c) 2009 Operational Dynamics Consulting Pty Ltd, and >> Others >> > > The correct change is to make it "2007-2009". > > >> + /** >> + * Set the orientation of the progress bar >> + * Four possible values: >> + * >> + * LEFT_TO_RIGHT >> + * RIGHT_TO_LEFT >> + * TOP_TO_BOTTOM >> + * BOTTOM_TO_TOP >> > > If you run javadoc over this (ie, do `make doc`) you will discover that > whitespace is not preserved (it is, to all intents, HTML) and so you'll > need to make that an <ul> list or use <br> if you want to have four > items. > > I'm not really sure you need to list them, but whatever. > http://java-gnome.sourceforge.net/4.0/doc/api/org/gnome/pango/Style.html > is an example of where we were verbose about it. > > You really ought to preview in a web browser. Like I said, > > $ make doc > > >> + * @param orientation >> + * a ProgressBarOrientation value >> > > You don't need that. > > >> + /** >> + * Get the current progress bar orientation >> > > if you're going to talk about the proper name of a class, then use the > class name: > > * Get orientation currently in effect in this ProgressBar. > > or whatever. Perhaps Kenneth will volunteer to help you with the prose. > > ++ > > You also really need to run the code formatter before committing. Serkan > can help you with that, but if you're not in Eclipse: > > $ make format > > and if you are, Source -> Format. > > AfC > Sydney > > > ------------------------------------------------------------------------ > > ------------------------------------------------------------------------------ > This SF.net email is sponsored by: > SourcForge Community > SourceForge wants to tell your story. > http://p.sf.net/sfu/sf-spreadtheword > ------------------------------------------------------------------------ > > _______________________________________________ > java-gnome-hackers mailing list > jav...@li... > https://lists.sourceforge.net/lists/listinfo/java-gnome-hackers > |
From: Kenneth P. <ken...@gm...> - 2009-01-19 04:41:18
Attachments:
signature.asc
|
On Sat, 17 Jan 2009 16:15:27 +0100 Goundy <go...@gm...> wrote: > Hi. > > Thank you Afc for your hints I corrected all what you pointed to. > And sorry for the time I put to do it but recently I got really hard > exams but fortunately I succeeded. > (Two more very easy exams and I'm done :-)) > > Well I hope this patch is better than the first one. > If there's more changes I should do, just reply back :-) > > Thanks and have a good day > > <snip> You forgot to attach the bundle, please do so that we may review it. |
From: Goundy <go...@gm...> - 2009-01-19 15:41:51
Attachments:
progressbarorientation.patch
|
Kenneth Prugh wrote: > On Sat, 17 Jan 2009 16:15:27 +0100 > Goundy <go...@gm...> wrote: > > >> Hi. >> >> Thank you Afc for your hints I corrected all what you pointed to. >> And sorry for the time I put to do it but recently I got really hard >> exams but fortunately I succeeded. >> (Two more very easy exams and I'm done :-)) >> >> Well I hope this patch is better than the first one. >> If there's more changes I should do, just reply back :-) >> >> Thanks and have a good day >> >> <snip> >> > > You forgot to attach the bundle, please do so that we may review it. > > ------------------------------------------------------------------------ > > ------------------------------------------------------------------------------ > This SF.net email is sponsored by: > SourcForge Community > SourceForge wants to tell your story. > http://p.sf.net/sfu/sf-spreadtheword > ------------------------------------------------------------------------ > > _______________________________________________ > java-gnome-hackers mailing list > jav...@li... > https://lists.sourceforge.net/lists/listinfo/java-gnome-hackers > Ew Sorry -_-" |
From: Andrew C. <an...@op...> - 2009-01-20 04:47:16
|
On Mon, 2009-01-19 at 16:40 +0100, Goundy wrote: > > You forgot to attach the bundle, please do so that we may review it. > Sorry And while we're at it, can you maybe try and quote a little less text when replying to emails on the list? We all got the original message. No need to quote it all back. AfC Hobart |
From: Andrew C. <an...@op...> - 2009-01-20 21:29:30
|
On Mon, 2009-01-19 at 16:40 +0100, Goundy wrote: > + /* > + * Perform progress from left to right > + */ So attention to detail is important. In this case, if you had previewed your work in Eclipse's JavaDoc window, or run `make doc` and then previewed the result in a web browser, you might have noticed that this is not JavaDoc. Fixed. Patch merged. AfC Hobart |