[PATCH] JMC-6180: Changing the Java source editor font changes the size of some values in the JMC tables

[Marcus Hirt]

There seem to be uitest classes that do contain NON-NLS tags. 
I will open a bug to clean this up.

Kind regards,
Marcus

On 2018-12-11, 19:11, "Elliott Baron" <ebaron at redhat.com> wrote:

    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