On Tue, 25 Jul 2023 12:50:35 GMT, Alexey Ivanov <[email protected]> wrote:
>> Tejesh R has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Spacing fix
>
> test/jdk/javax/swing/JTableHeader/JTableHeaderLabelRightAlignTest.java line
> 85:
>
>> 83: final JTableHeader header = table.getTableHeader();
>> 84: TableCellRenderer renderer = header.getDefaultRenderer();
>> 85: header.setDefaultRenderer(renderer);
>
> This doesn't make sense to me: you get the renderer from the header and set
> it back. Nothing's changed here, is it?
Yeah, not required.
> test/jdk/javax/swing/JTableHeader/JTableHeaderLabelRightAlignTest.java line
> 108:
>
>> 106: for (int i = 1; i < imgHeader.getHeight()-3; i++) {
>> 107: for (int j = verticalLineCol; j < verticalLineCol + 1; j++)
>> {
>> 108: if (expectedRGB != imgHeader.getRGB(j, i)) {
>
> Suggestion:
>
> for (int i = 1; i < imgHeader.getHeight()-3; i++) {
> for (int j = verticalLineCol; j < verticalLineCol + 1; j++) {
> if (expectedRGB != imgHeader.getRGB(j, i)) {
>
> These are coordinates, so name them as `x` and `y`. Adding the coordinates
> would help an engineer dealing with the test failure the clue what went
> wrong, that engineer could be you.
>
> The height should also be scaled: `(int) (imgHeader.getHeight() * SCALE - 3);`
For height, I am using BufferedImage height, not TableHeader height. So I guess
it is fine.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274535701
PR Review Comment: https://git.openjdk.org/jdk/pull/14464#discussion_r1274537494
