IBMStreams / streamsx.json

Toolkit for working with JSON in SPL applications.
http://ibmstreams.github.io/streamsx.json/
Other
3 stars 19 forks source link

NL Messages Files Location #74

Closed chanskw closed 4 years ago

chanskw commented 7 years ago

I am wondering if the NL messages files are at the right place and if the Messages class is written correctly.

I am seeing the *.properties files checked into the main source tree

There is a set here: https://github.com/IBMStreams/streamsx.json/tree/master/com.ibm.streamsx.json/impl/java/src/com/ibm/streamsx/json

Another set here: https://github.com/IBMStreams/streamsx.json/tree/master/com.ibm.streamsx.json/impl/java/src/com/ibm/streamsx/json/converters

In my experience, properties files are supposed to be separated into its own resource directory so that they are easy to locate and maintain. Putting them in the source tree makes the code base messy. And they are harder to update for future translation effort.

This is a guideline where we have found on how to do translation: https://examples.javacodegeeks.com/core-java/util/resourcebundle/java-resourcebundle-example/

I am also unsure if we are getting the messages correctly from this implementation of the Messages class. I expect a single Message class that would go into the resource directory to locate the bundle. We now have multiple Message class in each of the sub-directories in the project.

schubon commented 7 years ago

Hi Samantha, normally I prefer to have the resources in in one place probably in a namespace/directory containing the 'i18n' name. Unfortunately, someone decided to have the code in one project and to pack it into different JAR files, as you can see in the 'jar' and 'converter-jar' targets of the build script. If I had put it into one resource bundle then I would have created a dependency between the two archives for resources, too, or every archive would need to carry the resources of the other component in its archive (and we probably would start a discussion about why all these key/value pairs that aren't used by the component are delivered anyways). Furthermore, I thought packing the resources into a resource bundle (a third bundle) to be used by the other two, to be far over the top for these rather primitive projects.

If the classes ever become more complex I surely will refactor them, but the first action will be to move the converter classes into an own project.

Kind regards Norbert

chanskw commented 7 years ago

I am not sure I understand the problem, butI will study the build script a bit later. The converter jar file is for the tool that get used to generate schema. I have also thought that we should have a single jar. If that's keeping us from doing things correctly, then perhaps we should discuss refactor the project to a single jar.

I am also very concerned that each of the toolkit projects are handled differently. From what I can see, we put files in different places and seem to take a different approach depending on which project I am looking at. I think that we should have a single and consistent approach for all toolkit projects.

schubon commented 7 years ago

What I normally did for the Java resources was to introduce a package/directory under the toolkit namespace, like so: com.ibm.streams.toolkitname.i18n. That is how I normally do it, got used to that way years ago before IBM even acquired our company. The toolkits that got my name attached to them are: toolkit.cep, toolkit.fm, toolkit.rules, toolkit.rulescompiler, streamsx.iot, streamsx.json, streamsx.messaging

I varied the above method for streamsx.json (as discussed above) and streamsx.messaging. The latter feels more like a bunch of toolkits stuffed in a meta toolkit and there is already a discussion ongoing to split these toolkits (because a customer needed to support different versions of RabbitMQ and Kafka than had been bundled in the Messaging toolkit). Thus I decided to put the resources to the appropriate "sub" toolkit for easier refactoring when the discussion settles (I then expected that we will have a split soon).

rnostream commented 4 years ago

No change here.