Opinions

[Marcus Hirt]

Hi Mario,

I think the tags have a value - for most bundles most strings should
be externalized, and getting a warning about it helps making sure
that the strings are indeed externalized, especially for newcomers
to the project. Having the NON-NLS tag shows that the developer made 
a conscious decision about not externalizing the string.

For test bundles the strings should normally NOT be externalized,
hence (were it not for other considerations) I think we should 
neither warn, nor need the macro. In other words, we would be 
disabling a signal that carries no information (noise).

The other considerations in this case would be introducing project
specific warning/error settings for test bundles, and perhaps 
confusing people about when to have and when to not have the tags.

I think the two choices to consider are:
* Keep using the NON-NLS tags in the test bundles
  (few project specific overrides (any existing), 
   pointless tags in the test bundles)
* Stop using the NON-NLS tags in the test bundles
  (test project specific overrides,
   no tags in the test bundles)

I have no strong opinion either way either. The pros and cons are 
distributed pretty evenly for me, with maybe a slight anti-noise 
bias. Does anyone else have an opinion in the matter?

Kind regards,
Marcus

On 2019-02-22, 12:01, "Mario Torre" <neugens at redhat.com> wrote:

    On Thu, 2019-02-21 at 19:14 +0100, Marcus Hirt wrote:
    > At the same time it's annoying to get externalization warnings
    > for things that should never be externalized. That said,
    > I agree local project overrides should only be used when necessary, 
    > and perhaps this is not necessary. Also something can be said
    > for keeping the rules across all the projects the same, i.e. 
    > always use the macros or externalize.
    > 
    > For me it's a coin toss. Don't even remember who was annoyed at 
    > the macros in the test bundles anymore. ;)
    
    I don't really have a strong opinion here, so I guess what's more
    convenient for you. I would probably leave things as they are and try a
    best effort on keeping the code clean, or if the NON-NLS keeps warnings
    at bay and works weel, let's just leave it like that?
    
    Personally, I always found this enforcement from Eclipse to be tedious,
    I understand where it comes from but the need for this extra comment
    seems an hack and doesn't really tell me as programmer much (I can see
    from the line if it's meant to be localised or not) and the amount of
    warning is just a lot of noise. So once again, what you feel it's best
    is surely the right answer ;)
    
    Cheers,
    Mario
    
    > Kind regards,
    > Marcus
    > 
    > 
    > On 2019-02-21, 17:06, "Mario Torre" <neugens at redhat.com> wrote:
    > 
    >     On Thu, 2019-02-21 at 15:49 +0100, Marcus Hirt wrote:
    >     > Hi all,
    >     > 
    >     > The tests are normally not locale dependent, and so should not
    >     > require externalization. Thus, it doesn’t make much sense to
    > require
    >     > using the NON-NLS macros in pure test bundles. That said,
    > changing
    >     > the error reporting for non-externalized strings for the test
    > bundles
    >     > would require Eclipse project settings overrides for the test
    >     > bundles. But perhaps that is ok?
    >     > 
    >     > What do you say? Require use of NON-NLS in test bundles or not?
    >     
    >     I think it makes sense to have a global configuration rather than
    > a per
    >     test one to ignore the localisation string.
    >     
    >     Cheers,
    >     Mario
    >     
    >     -- 
    >     Mario Torre
    >     Associate Manager, Software Engineering
    >     Red Hat GmbH <https://www.redhat.com>
    >     9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898
    >     
    >     
    >     
    > 
    > 
    -- 
    Mario Torre
    Associate Manager, Software Engineering
    Red Hat GmbH <https://www.redhat.com>
    9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898