Hi all, there is a patch [1] removing `AppErrors.properties` from webadmin project to simplify edits of AppErrors/EngineMessage. AppErrors.properties from webadmin project will be merged to AppErrors.properties in frontend project. This requires some manual resolutions of conflicts of translation values. I'd like to kindly ask you to review altered translation values. They are mostly just typos. Thanks. Jakub [1]: https://gerrit.ovirt.org/#/c/76215/

On Tue, Jun 13, 2017 at 2:55 PM, Vojtech Szocs <vszocs(a)redhat.com&gt; wrote: > Hi Jakub, thanks for taking the effort to simplify AppErrors handling! > > The AppErrors.properties file and its localized variants should live in a > single location, e.g. frontend/webadmin/modules/frontend. We should > avoid having multiple AppErrors.properties copies spread across the > codebase. > Yes please! Why do we even have these three copies? I vaguely recall an explanation that there's a "default" properties file (the backend one?), and then the others can overrie specific keys (e.g., to make the user portal simpler and remove admin complexities) - but it doesn't seem as though the this is how it's used. > The backend dal (data access layer) module has its own copy of > AppErrors.properties file. One option is to copy it from the source > location during Engine build, another option is to simply use symlinks. > Why do even have this file? For REST API/SDK errors? > In any case, there should be a unit test that ensures all EngineMessage > enum members are reflected as methods in AppErrors interface. This will > give us the confidence that backend EngineMessage's have proper strings > associated with them. > We've had the opposite test for quite a long time: https://github.com/oVirt/ovirt-engine/commits/ 0c7d7e7101142b95bd78e76be08dbb121c0c90f7/frontend/webadmin/ modules/frontend/src/test/java/org/ovirt/engine/ui/ frontend/AppErrorsTest.java But in indeed did's have a test for missing methods in the AppErrors interface. I tried cooking up a simple test and found that we're missing several hundred(!) keys, so I just carpet-bomb added them all, to be sorted out later: https://gerrit.ovirt.org/#/c/78354/ However, I'm getting a weird GWT error when I try to build the application: [ERROR] Errors in 'gen/org/ovirt/engine/ui/frontend/com_gwtplatform_mvp_ client_DesktopGinjector_DesktopGinjectorGinjector_fragment.java' [INFO] [ERROR] Line 50: Failed to resolve 'org.ovirt.engine.ui.frontend.VdsmErrors' via deferred binding I'm obviously doing something stupid I shouldn't be doing, but I must admit I don't understand what I'm doing wrong. Some advice from our residenant GWT experts (yes Vojtech, I'm pointing at you :-)) woul be appreciated.

> Regards, > Vojtech > > > On Mon, Jun 12, 2017 at 8:51 PM, Jakub Niedermertl <jniederm(a)redhat.com&gt; > wrote: > >> Hi all, >> >> there is a patch [1] removing `AppErrors.properties` from webadmin >> project to simplify edits of AppErrors/EngineMessage. AppErrors.properties >> from webadmin project will be merged to AppErrors.properties in frontend >> project. This requires some manual resolutions of conflicts of translation >> values. >> >> I'd like to kindly ask you to review altered translation values. They >> are mostly just typos. >> >> Thanks. >> Jakub >> >> [1]: https://gerrit.ovirt.org/#/c/76215/ >> > > > _______________________________________________ > Devel mailing list > Devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/devel >

Oh, that was stupid on my side - there are indeed several (hundred) properties missing, so we can't merge this test. I wonder how we should push this forward. It isn't just a technicality of adding entries to the properties file, we need to know what the message should be (which I don't, at least not for all of them). Unless we want to just have "TBD" there. On Wed, Jun 21, 2017 at 9:33 AM, Allon Mureinik <amureini(a)redhat.com&gt; wrote: > > > On Wed, Jun 21, 2017 at 9:12 AM, Allon Mureinik <amureini(a)redhat.com&gt; > wrote: > >> >> >> On Tue, Jun 13, 2017 at 2:55 PM, Vojtech Szocs <vszocs(a)redhat.com&gt; >> wrote: >> >>> Hi Jakub, thanks for taking the effort to simplify AppErrors handling! >>> >>> The AppErrors.properties file and its localized variants should live in >>> a single location, e.g. frontend/webadmin/modules/frontend. We should >>> avoid having multiple AppErrors.properties copies spread across the >>> codebase. >>> >> Yes >> >> please! >> Why do we even have these three copies? >> I vaguely recall an explanation that there's a "default" properties file >> (the backend one?), and then the others can overrie specific keys (e.g., to >> make the user portal simpler and remove admin complexities) - but it >> doesn't seem as though the this is how it's used. >> >> >>> The backend dal (data access layer) module has its own copy of >>> AppErrors.properties file. One option is to copy it from the source >>> location during Engine build, another option is to simply use symlinks. >>> >> Why >> >> do even have this file? For REST API/SDK errors? >> >> >>> In any case, there should be a unit test that ensures all EngineMessage >>> enum members are reflected as methods in AppErrors interface. This will >>> give us the confidence that backend EngineMessage's have proper strings >>> associated with them. >>> >> We've had >> the opposite test for quite a long time: >> >> https://github.com/oVirt/ovirt-engine/commits/0c7d7e7101142b >> 95bd78e76be08dbb121c0c90f7/frontend/webadmin/modules/fronten >> d/src/test/java/org/ovirt/engine/ui/frontend/AppErrorsTest.java >> >> But in indeed did's have a test for missing methods in the AppErrors >> interface. >> I tried cooking up a simple test and found that we're missing several >> hundred(!) keys, so I just carpet-bomb added them all, to be sorted out >> later: >> https://gerrit.ovirt.org/#/c/78354/ >> >> However, I'm getting a weird GWT error when I try to build the >> application: >> >> [ERROR] Errors in 'gen/org/ovirt/engine/ui/front >> end/com_gwtplatform_mvp_client_DesktopGinjector_DesktopGinje >> ctorGinjector_fragment.java' >> [INFO] [ERROR] Line 50: Failed to resolve >> 'org.ovirt.engine.ui.frontend.VdsmErrors' via deferred binding >> >> I'm obviously doing something stupid I shouldn't be doing, but I must >> admit I don't understand what I'm doing wrong. >> Some advice from our residenant GWT experts (yes Vojtech, I'm pointing >> at you :-)) woul be appreciated. >> > > The > > weirest thing is that CI actually passes on this, but I get the > aforementioned error locally when I run > > $ mvn clean install -Pgwt-admin -Dgwt.userAgent=safari > > Now I'm sure I'm doing something stupid - just not sure what. > >> >> >>> Regards, >>> Vojtech >>> >>> >>> On Mon, Jun 12, 2017 at 8:51 PM, Jakub Niedermertl <jniederm(a)redhat.com >>> > wrote: >>> >>>> Hi all, >>>> >>>> there is a patch [1] removing `AppErrors.properties` from webadmin >>>> project to simplify edits of AppErrors/EngineMessage. AppErrors.properties >>>> from webadmin project will be merged to AppErrors.properties in frontend >>>> project. This requires some manual resolutions of conflicts of translation >>>> values. >>>> >>>> I'd like to kindly ask you to review altered translation values. They >>>> are mostly just typos. >>>> >>>> Thanks. >>>> Jakub >>>> >>>> [1]: https://gerrit.ovirt.org/#/c/76215/ >>>> >>> >>> >>> _______________________________________________ >>> Devel mailing list >>> Devel(a)ovirt.org >>> http://lists.ovirt.org/mailman/listinfo/devel >>> >> >> >