Re: [ovirt-devel] AppErrors.properties merged together

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 >>> >> >> >