[ovirt-devel] oVirt Engine checkstyle upgrade

Mon Mar 27 21:46:02 UTC 2017

On Mon, Mar 27, 2017 at 8:48 PM, Vojtech Szocs <vszocs at redhat.com> wrote:

>
>
> On Mon, Mar 27, 2017 at 2:10 PM, Allon Mureinik <amureini at redhat.com>
> wrote:
>
>>
>>
>> On Mon, Mar 27, 2017 at 3:09 PM, Allon Mureinik <amureini at redhat.com>
>> 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 at redhat.com>
>>> 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 at redhat.com> 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 at redhat.com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Fri, Mar 24, 2017 at 3:51 PM, Vojtech Szocs <vszocs at redhat.com>
>>>>>> 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 at redhat.com>
>>>>>>> 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 at redhat.com> 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 at ovirt.org
>>>>>>>>> http://lists.ovirt.org/mailman/listinfo/devel
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Devel mailing list
>>>>>> Devel at ovirt.org
>>>>>> http://lists.ovirt.org/mailman/listinfo/devel
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Devel mailing list
>>>>> Devel at ovirt.org
>>>>> http://lists.ovirt.org/mailman/listinfo/devel
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Greg Sheremeta, MBA
>>>> Red Hat, Inc.
>>>> Sr. Software Engineer
>>>> gshereme at redhat.com
>>>>
>>>> _______________________________________________
>>>> Devel mailing list
>>>> Devel at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/devel
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20170328/67d94de0/attachment-0001.html>