a few comments on the comments: re: Catch blocks - multi-catches. Recently Joern submitted a big patch for something similar: try with resources.
I think he did this using some refactoring tool, perhaps part of Intellij Idea framework. Using tools is usually better than doing things by hand :-) (more reliable) ------------ re: need to install UIMA plugins first. I typically don't do that! Instead I tell Eclipse where to find the uima plugins, by going to the "Manage" button on the top level install new software panel, and just adding the main UIMA site (https://www.apache.org/dist/uima/eclipse-update-site/uima ). This has the effect of enabling the Ruta install to fetch just those OSGi bundles it depends on. Of course, it's also a good test to first install the UIMA plugins... =========== Re: "This is normal behavior since you most likely did not install the IDE extensions Eclipse plugins for these language elements." Is there a reason these extensions are not automatically installed with everything else? It's nice to be able to eliminate user's encounters with error situations they would not expect. =========== Re: whether or not to release anyways. I know all releases still have bugs. The main reasons to do another RC would be (in my opinion) - if these issues would likely "hit" users of Ruta, and therefore lead to perhaps a somewhat negative perception about "quality" and - if fixing (most of) them would not be a big effort. Peter, how would you assess these? Of course, doing another RC means another cycle of testing, but by comparing (diff-ing) with this RC, the review might go faster... -Marshall On 2/10/2019 4:20 PM, Peter Klügl wrote: > Hi, > > > thank you for the extensive review :-) > > > Comments below... > > > Am 09.02.2019 um 00:49 schrieb Richard Eckart de Castilho: >> Trying to build with Java "11.0.1" 2018-10-16 LTS... >> >> Result: Fails when japicmp-maven-plugin is invoked >> Severity: not a blocker >> Comment: Consider configuring the Maven enforcer plugin such that >> it checks that a Java 8 is being used to compile - or - >> fix the build by copying the required configurations from >> the UIMA core framework build. Dependency on Java 8 not >> mentioned in the README file in the "Building from the >> Source Distribution" section > > Yes, nothing happended yet towards Java 11. > > >> Trying to build with Java "1.8.0_181"... >> >> Result: Build successful >> >> Downloaded source ZIP >> >> Built ruta-maven-example >> >> Result: Build successful >> >> Built ruta-ep-example-extensions >> >> Result: Build successful >> >> Check ruta-tutorial-GermanNovels "examples" project >> >> Result: contains only latex source code >> Severity: not a blocker >> Comment: It would be more useful if a pre-compile PDF would >> be included in the repo, e.g. in the "GermanNovels" examples >> folder. Having the latex sources under the examples folder seems >> odd. > > I wonder if this tutorial caused more problems than it helped. I am > really thinking about removing it... > > It does not present current best practices. > > >> Checking DEPENDENCIES file >> >> Result: No issue found >> >> Checking README file >> >> Result: ok >> Comment: The README file mentions the uimaFIT JCasGenPomFriendly >> which doesn't exist anymore since quite some time. >> Empty line missing before "mvn clean install" > > Yep, I'll remove that. > > >> Checking ruta-2.7.0-source-release.zip.sha512 >> >> Result: ok >> >> Spot checking code changes >> >> Result: ok >> Comment: Many catch blocks could be consolidated using Java 8 >> multi-catches. >> Ample opportunities for cleaner code up using lambdas. > Oh yes. > > >> The language syntax documentation says e.g. `("<-" "{" >> SimpleStatement+ "}")*` >> but shouldn't that be a `+` in the end (i.e. 1-or-more)? > > This is the gammar rule for inlined rules as condition, right? The star > quantifier is correct, the inlined rule block is optional. > > >> Some unit test methods declare excessive lists of thrown >> exceptions. This >> can be simplified by just declaring `throws Exception` on the >> unit test >> method. Doing so on @Test methods is not harmful or bad style. > > I did a small refactoring with the effect that I added another > exception. I was actually thinking about replacing them, but I was just > too lazy. > > > I normally try to improve the code when I touch it. Maybe I will find > some time to improve it overall and use some more up to date Java stuff. > > >> Installed Ruta from update site into a freshly downloaded Eclipse >> >> Result: ok >> Comment: needed to install UIMA Eclipse plugins first > > This is only due to the RC update site which does not contain the other > UIMA bundles. > > >> Imported example projects into Eclipse >> >> Result: errors >> Comment: I pointed "Import -> Existing Projects" at the examples folder >> contained in the source ZIP. Several projects are imported. >> The "Extensions Example" and "GermanNovels" projects have errors. >> "GermanNovels" is a Maven project, but is not imported as such. >> Even when importing it via "Import -> Existing Maven Projects", >> it has errors. >> >> The "ruta-maven-example" fails (I run Eclipse on Java 11): >> >> Execution descriptors of goal >> org.apache.uima:ruta-maven-plugin:2.7.0:generate failed: Failed to read >> candidate component class: URL >> [jar:file:/Users/bluefire/.m2/repository/org/apache/uima/ruta-core/2.7.0/ruta-core-2.7.0.jar!/org/apache/uima/ruta/resource/XMLEventHandler.class]; >> nested exception is org.springframework.core.NestedIOException: ASM >> ClassReader failed to parse class file - probably due to a new Java class >> file version that isn't supported yet: class path resource >> [org/xml/sax/helpers/DefaultHandler.class]; nested exception is >> java.lang.IllegalArgumentException >> (org.apache.uima:ruta-maven-plugin:2.7.0:generate:descriptors:process-classes) >> >> It seems as if an old version of ASM is used to analyze classes >> from the Java 11 JDK (which ASM does not understand yet). > > (builds with Java 8) > > >> The "ruta-example-german-novels" fails with: >> >> error: Feature "language" is not defined for type >> "de.tudarmstadt.ukp.dkpro.core.api.segmentation.type.Document". >> >> I guess that should be >> "de.tudarmstadt.ukp.dkpro.core.api.metadata.type.DocumentMetaData"? > > This example project is broken: > https://issues.apache.org/jira/browse/UIMA-4954 > > The error could be a false postive (bug in the editor, could work > anyway). It should actually be resolved to DocumentAnnotation. > > >> The "ExtensionsExample" fails with several messages: >> >> - error: Action "ExampleAction" is not defined. >> - error: Condition "" is not defined. >> - Mismatched Input: : } >> - Mismatched Input: Expecting "SEMI" but found "{". >> - Type "REVERSE " not defined in this script/block! > > This is normal behavior since you most likely did not install the IDE > extensions Eclipse plugins for these language elements. This project > just provides a test case, if someone wants to test a different project > (ruta-ep-example-extensions). Marshall had the same problem several > reviews ago IIRC. > > >> I think it would be a good idea to update the examples and ensure they work? > > In my opinion, the example projects are not intended to be ready to use > projects but rather examples in a documentary way how one could do > something in that direction, e.g. set up a maven build for a ruta > project. Ignoring the German Novels example due to the open ticket for > it, it comes down to the build problems with Java 11. > > > It would be ok for me to release rc2 and mention the build problems in > the announce mail. I already created a ticket for it, and the user on > Stackoverfow was also able to resolve his/her ASM problem. > > > What do you think? > > > Best, > > > Peter > > >> Cheers, >> >> -- Richard >> >>
