SV: [PATCH] JMC-6180: Changing the Java source editor font changes the size of some values in the JMC tables
Elliott Baron
ebaron at redhat.com
Tue Dec 11 18:11:41 UTC 2018
Hi Marcus,
On 2018-12-06 4:54 p.m., Marcus Hirt wrote:
> Quite frankly, we shouldn't need to, but I think we currently do
> this across all code.
It looks like at least the UI tests don't use NON-NLS tags. This seems
to be intentional since the checked-in Eclipse settings are set to
ignore non-externalized string literals [1]:
> eclipse.preferences.version=1
> org.eclipse.jdt.core.compiler.problem.nonExternalizedStringLiteral=ignore
Should I add the tags to my test for this patch, or is the latest patch
good to go?
Thanks,
Elliott
[1]
http://hg.openjdk.java.net/jmc/jmc/file/abb4b7f54009/application/uitests/org.openjdk.jmc.browser.uitest/.settings/org.eclipse.jdt.core.prefs#l2
>
> Kind regards,
> Marcus
>
> -----Ursprungligt meddelande-----
> Från: jmc-dev <jmc-dev-bounces at openjdk.java.net> För Elliott Baron
> Skickat: den 6 december 2018 21:10
> Till: Marcus Hirt <marcus.hirt at oracle.com>; jmc-dev at openjdk.java.net
> Ämne: Re: [PATCH] JMC-6180: Changing the Java source editor font changes the size of some values in the JMC tables
>
> Hi Marcus,
>
> On 2018-12-06 1:34 p.m., Marcus Hirt wrote:
>> Hi Elliott,
>>
>> Just one nit - don't forget to put //$NON-NLS-1$ tags on string
>> constants that should not be localized, e.g.:
>>
>> private static final String FIXED_TEXT_FONT =
>> "org.openjdk.jmc.fixedtextfont"; //$NON-NLS-1$
>>
>> Looks fine - don't need another review after fixing this.
>>
>> Thank you for your contribution!
>>
>> Kind regards,
>> Marcus
>
> Thanks for the review!
>
> I have added the missing NON-NLS tag (and set Eclipse to warn me in the future). Am I correct that these tags are not required for test classes?
>
> Thanks,
> Elliott
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jmc-6180-v2.patch
Type: text/x-patch
Size: 10369 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/jmc-dev/attachments/20181211/ad0f97a8/jmc-6180-v2-0001.patch>
More information about the jmc-dev
mailing list