Re: [ovirt-devel] oVirt Engine checkstyle upgrade

On Mon, Mar 27, 2017 at 8:48 PM, Vojtech Szocs <vszocs(a)redhat.com&gt; wrote: > > > On Mon, Mar 27, 2017 at 2:10 PM, Allon Mureinik <amureini(a)redhat.com&gt; > wrote: > >> >> >> On Mon, Mar 27, 2017 at 3:09 PM, Allon Mureinik <amureini(a)redhat.com&gt; >> wrote: >> >>> Indeed, Greg.thanks >>> >>> As a temporary solution, you could cherry-pick https://gerrit.ovi >>> rt.org/#/c/74611 and work above it. >>> It should solve the issue (even though IMHO it isn't ready for merging >>> to master). >>> >>> Vojta - are you updating https://gerrit.ovirt.org/#/c/74611 as per my >>> comments there, or should I take it over? >>> >> I literally have no idea who I did made this typo. >> I meant "Vojtech", of course.​ >> > > ​In Czech language, Vojta actually means Vojtech, in a less formal way =) > ​In that case, I totally meant to type that :-) ​ > > I've just updated https://gerrit.ovirt.org/#/c/74611/ according to your > comments. > ​I cleaned up some white-spacing noise there that seemed unrelated to the patch and merged.

From my local testing, using -Pgwtdev now works just find. Votech (or Vojta, if you prefer ^_^) - many thanks for your efforts here. Marek, and everyone else - please let me know if this still causes you any grief.​ > > The reason why GWT debugger fails to start is because one of the > non-localized strings (which comes from `gwt-extension` and therefore > shouldn't be checked in the first place..) contains a {} placeholder, > NlsCheck logs it, and eventually crashes because it thinks it's a log > message placeholder, but it's just something we want printed out. > > >> >> >> >>> >>> On Mon, Mar 27, 2017 at 3:03 PM, Greg Sheremeta <gshereme(a)redhat.com&gt; >>> wrote: >>> >>>> From the comments in 74619 >>>> >>>> "" " >>>> So, tl;dr - it /won't/ work with this patch but without 74611. This >>>> patch should be applied before 74611 >>>> "" " >>>> >>>> On Mon, Mar 27, 2017 at 7:42 AM, Marek Libra <mlibra(a)redhat.com&gt; >>>> wrote: >>>> >>>>> With >>>>> https://gerrit.ovirt.org/#/c/74619/ >>>>> >>>>> applied, following is still failing: >>>>> make gwt-debug DEBUG_MODULE=webadmin DEV_EXTRA_BUILD_FLAGS_GWT_DEFA >>>>> ULTS="-Dgwt.cssResourceStyle=pretty -Dgwt.userAgent=gecko1_8" >>>>> >>>>> with message: >>>>> [WARNING] The requested profile "gwt-user" could not be activated >>>>> because it does not exist. >>>>> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.17:check >>>>> (checkstyle) on project webadmin: Failed during checkstyle configuration: >>>>> Exception was thrown while processing /home/mlibra/IdeaProjects/ovir >>>>> t-engine/frontend/webadmin/modules/gwt-extension/src/main/ja >>>>> va/org/ovirt/engine/ui/uioverrides/org/slf4j/Logger.java: can't >>>>> parse argument number: For input string: "" -> [Help 1] >>>>> >>>>> >>>>> Any hint, please? >>>>> Marek >>>>> >>>>> >>>>> >>>>> On Fri, Mar 24, 2017 at 4:14 PM, Vojtech Szocs <vszocs(a)redhat.com&gt; >>>>> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Fri, Mar 24, 2017 at 3:51 PM, Vojtech Szocs <vszocs(a)redhat.com&gt; >>>>>> wrote: >>>>>> >>>>>>> Found the problem after debugging NlsCheck. >>>>>>> >>>>>>> First of all, it checks all kinds of Java sources, including the >>>>>>> generated ones. That's silly and one of the reasons why Checkstyle >>>>>>> execution takes a rather long time. I'll fix that. >>>>>>> >>>>>>> Next, when checking a Java source that contains string "{}" >>>>>>> (without quotes) it will log the problem, but Checkstyle message logging >>>>>>> infra things that "{}" is a placeholder to resolve, but it's not, and it >>>>>>> fails on NumberFormatException. I'll fix that too. >>>>>>> >>>>>> >>>>>> ​https://gerrit.ovirt.org/#/c/74611/​ >>>>>> >>>>>> >>>>>>> Vojtech >>>>>>> >>>>>>> >>>>>>> On Fri, Mar 24, 2017 at 3:19 PM, Vojtech Szocs <vszocs(a)redhat.com&gt; >>>>>>> wrote: >>>>>>> >>>>>>>> Hi Allon, >>>>>>>> >>>>>>>> I think I found some strange Checkstyle related problems on master. >>>>>>>> >>>>>>>> Engine build with (GWT compilation enabled) works OK. >>>>>>>> >>>>>>>> Next, trying to start GWT debugger: >>>>>>>> >>>>>>>> $ make gwt-debug DEBUG_MODULE=webadmin \ >>>>>>>> DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS="-Dgwt.userAgent=gecko1_8,safari" >>>>>>>> \ >>>>>>>> DEV_EXTRA_BUILD_FLAGS="-Dgwt.logLevel=INFO -Dgwt.locale=en_US >>>>>>>> -Dgwt.compiler.localWorkers=1" \ >>>>>>>> DEV_BUILD_GWT_SUPER_DEV_MODE=1 >>>>>>>> >>>>>>>> maven-checkstyle-plugin:check execution fails on >>>>>>>> >>>>>>>> frontend/webadmin/modules/gwt-extension/src/main/java/org/ov >>>>>>>> irt/engine/ui/uioverrides/org/ovirt/engine/core/compat/Forma >>>>>>>> tterDotnet.java >>>>>>>> can't parse argument number: (\\d)\\: For input string: "(\\d)\\" >>>>>>>> >>>>>>>> the class isn't used, removed it, retry. Now it fails on: >>>>>>>> >>>>>>>> frontend/webadmin/modules/gwt-extension/src/main/java/org/ov >>>>>>>> irt/engine/ui/uioverrides/org/slf4j/Logger.java >>>>>>>> can't parse argument number: For input string: "" >>>>>>>> >>>>>>>> I guess it's a bug in our NON-NLS check? But why doesn't the >>>>>>>> problem occur during Engine build? >>>>>>>> >>>>>>>> I'm thinking about disabling Checkstyle for gwt-extension module, >>>>>>>> as it contains custom GWT RPC serializers and GWT class overrides, and >>>>>>>> maybe the file path src/main/java/org/ovirt/engine >>>>>>>> /ui/uioverrides/here/goes/actual/pkg is confusing the Checkstyle >>>>>>>> now. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Vojtech >>>>>>>> >>>>>>>> >>>>>>>> On Wed, Mar 22, 2017 at 10:33 PM, Allon Mureinik < >>>>>>>> amureini(a)redhat.com&gt; wrote: >>>>>>>> >>>>>>>>> Hi all, >>>>>>>>> >>>>>>>>> As per [1], I've just merged a series of patches that upgrades >>>>>>>>> the oVirt engine to use the latest maven-checkstyle-plugin and checkstyle >>>>>>>>> packages. >>>>>>>>> >>>>>>>>> Please note that the newer checkstyle is a tad stricter than the >>>>>>>>> old one we used to have (read: it contains several fixes for bugs where the >>>>>>>>> old checkstyle was supposed to find issues but missed them). >>>>>>>>> I also took the opportunity and added a couple of new checks that >>>>>>>>> enforce rules we were de-facto adhering to anyway. >>>>>>>>> >>>>>>>>> If any problems come up, please let me know. >>>>>>>>> >>>>>>>>> >>>>>>>>> -Your friendly neighborhood cleanup dude >>>>>>>>> >>>>>>>>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1433408 >>>>>>>>> >>>>>>>>> _______________________________________________ >>>>>>>>> Devel mailing list >>>>>>>>> Devel(a)ovirt.org >>>>>>>>> http://lists.ovirt.org/mailman/listinfo/devel >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> Devel mailing list >>>>>> Devel(a)ovirt.org >>>>>> http://lists.ovirt.org/mailman/listinfo/devel >>>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> Devel mailing list >>>>> Devel(a)ovirt.org >>>>> http://lists.ovirt.org/mailman/listinfo/devel >>>>> >>>> >>>> >>>> >>>> -- >>>> Greg Sheremeta, MBA >>>> Red Hat, Inc. >>>> Sr. Software Engineer >>>> gshereme(a)redhat.com >>>> >>>> _______________________________________________ >>>> Devel mailing list >>>> Devel(a)ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/devel >>>> >>> >>> >> >