Bug 235626 - [releng] Get rid of Resource Bundles, use Eclipse Message Bundles instead
Summary: [releng] Get rid of Resource Bundles, use Eclipse Message Bundles instead
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P1 normal (vote)
Target Milestone: 3.0 RC3   Edit
Assignee: Martin Oberhuber CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 235644 235600 235718
  Show dependency tree
 
Reported: 2008-06-04 11:40 EDT by Martin Oberhuber CLA
Modified: 2008-06-04 18:50 EDT (History)
0 users

See Also:
mober.at+eclipse: review? (ddykstal.eclipse)


Attachments
Patch fixing the trivial occurrences (16.32 KB, patch)
2008-06-04 12:24 EDT, Martin Oberhuber CLA
no flags Details | Diff
Patch fixing terminals.ui (13.68 KB, patch)
2008-06-04 13:15 EDT, Martin Oberhuber CLA
no flags Details | Diff
Patch fixing examples (48.79 KB, patch)
2008-06-04 17:25 EDT, Martin Oberhuber CLA
mober.at+eclipse: review+
Details | Diff
Patch addressing dstore.security (67.26 KB, patch)
2008-06-04 17:34 EDT, Martin Oberhuber CLA
mober.at+eclipse: review+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2008-06-04 11:40:31 EDT
An item on the http://wiki.eclipse.org/Ganymede#Must_Do is to use Eclipse Message Bundles so we should get rid of all Resource Bundle related code.

Affected plugins:
* org.eclipse.rse.examples.tutorial
* org.eclipse.rse.terminals.ui
* org.eclipse.dstore.security
* org.eclipse.rse.processes.ui

This change is breaking API in some places, as we're getting rid of Activator.getString() or Activator.getResourceBundle() in some places where Activator is public API.

-----------Enter bugs above this line-----------
TM 3.0RC2 testing
------------------------------------------------
Comment 2 Martin Oberhuber CLA 2008-06-04 12:24:12 EDT
Created attachment 103589 [details]
Patch fixing the trivial occurrences

Attached patch fixes the trivial occurrences where just comments or dead code are removed and no API is broken. I'm committing this right away:

[235626][cleanup] Get rid of resource bundle related dead code
Comment 3 Martin Oberhuber CLA 2008-06-04 13:15:02 EDT
Created attachment 103598 [details]
Patch fixing terminals.ui

Attached patch fixes terminals.ui plugin (also no API change).
Comment 4 David Dykstal CLA 2008-06-04 17:13:27 EDT
+1 on the terminals.ui patch -- Dave
Comment 5 Martin Oberhuber CLA 2008-06-04 17:25:20 EDT
Created attachment 103653 [details]
Patch fixing examples

Attached patch fixes Examples to use Eclipse MessageBundle rather than Java ResourceBundle. This is theoretically breaking API change, because there used to be RSESamplesPlugin.getResourceBundle() in an API package; but since the Examples plugin doesn't export ANYTHING from its Manifest it is not breaking.

I still figured that this would be a good chance to have an argument for upreving Examples version number to 3.0 in the bundle as well as the feature, thus addressing one question from bug 235600.

Note that HTML Docs in the rse.doc.isv, which include the Example code, will need to get updated after this change as per bug 235644. I still like the change a lot, because it allows for static compile-time checking of resource strings, and brings the code up-to-date.
Comment 6 Martin Oberhuber CLA 2008-06-04 17:34:52 EDT
Created attachment 103654 [details]
Patch addressing dstore.security

Here is the final patch for the dstore.security plugin. This was a particularly wicked one, because the NLS keys had been in the UniversalSecurityProperties.properties file already, and there had even existed an NLS access class, but the code had accessed the Strings wrongly. It was by pure chance that this worked.

This change is also not affecting any API since all code is "internal". I did detect some missing NLS keys, though, when making the change; because the new MessageBundle approach allows for besser compile-time error checking. I filed bug 235718 for that issue.

This concludes my work for this bug. DaveD, please review, thanks!
Comment 7 Martin Oberhuber CLA 2008-06-04 17:37:10 EDT
(In reply to comment #4)
Committing the terminals.ui patch as per Dave's positive Review on comment #4:

[235626] Convert terminals.ui to MessageBundle format

Comment 8 David Dykstal CLA 2008-06-04 18:16:07 EDT
+1 on the examples. It did change the keys in the properties file, but examples are not translated so this is not a problem.
Comment 9 Martin Oberhuber CLA 2008-06-04 18:40:40 EDT
Comment on attachment 103653 [details]
Patch fixing examples

Examples committed as per comment #8:

[235626] Convert examples to MessageBundle format
Comment 10 David Dykstal CLA 2008-06-04 18:42:08 EDT
+1 on the dstore.security patch.

Thanks for finding the missing strings here and opening the other bug. I'll have DaveM furnish the correct values for RC4.
Comment 11 Martin Oberhuber CLA 2008-06-04 18:49:19 EDT
Comment on attachment 103654 [details]
Patch addressing dstore.security

Committing dstore.security patch as per comment #10:

[235626] Convert dstore.security to MessageBundle format
Comment 12 Martin Oberhuber CLA 2008-06-04 18:50:32 EDT
This completes work on this bug. Dave, if you could please put your Review+ flag on the upper right corner here on the bug to officially complete things.