I can't see the changes being a problem to implement.

I was hoping you guys would run some kind of automatic format tool across the 
patch so I didn't try and keep it consistent with what the project uses. I 
don't use Eclipse so I will just try to get it into the expected format before 
I submit any future patches.

On Sat, 19 Sep 2009 12:53:44 am Greg Brown wrote:
> Scott,
>
> Nice work. I have a few comments:
>
> - I suggest that we take an approach similar to how preferred width
> and height are bounded in Component. Rather than firing two separate
> events for min. and max. width changes, we would fire a single
> "columnWidthLimitsChanged(Component component, int
> previousMinimumWidth, int previousMaximumWidth)". We would define a
> TableView.Column#setColumnWidthLimits() method to which setMinimumWidth
> () and setMaximumWidth() would both delegate.
>
> - Coding style:
>    - Opening curly brace should be on the same line as the method
> name, if statement, etc. rather than on the next line.
>    - There should be no spaces after opening paren or before closing
> paren.
>    - Javadoc should begin on the first "*" line, not the "/**" line.
>    - A space should be included after a comma, but not before.
>    - Abbreviations should be avoided (e.g. prefer "previousWidth" to
> "prevWidth").
>
> If you are using Eclipse as an editor, you can use the Pivot code
> style template to automatically format your code:
>
> http://cwiki.apache.org/confluence/download/attachments/108483/pivot_style.
>xml
>
> If you can make these changes and send out an updated patch, I will
> apply it and test it out. Assuming that everything works as expected,
> I'll go ahead and submit it.
>
> Thanks!
> Greg
>
> On Sep 17, 2009, at 9:27 PM, Scott Lanham wrote:
> > Hi Guys,
> >
> > Attached is a patch that implements the Minimum/Maximum column width
> > properties. I don't know that it is as correct as it should be but I
> > am sure
> > that can be fixed :-)
> >
> > Cheers,
> >
> > Scott.
> > <pivot_svn_patch_20090918_01.diff>

Reply via email to