You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@opennlp.apache.org by "kinow (via GitHub)" <gi...@apache.org> on 2023/01/30 16:48:12 UTC

[GitHub] [opennlp-sandbox] kinow commented on a diff in pull request #79: Introduce common parent pom for all 'opennlp-sandbox' components

kinow commented on code in PR #79:
URL: https://github.com/apache/opennlp-sandbox/pull/79#discussion_r1090875982


##########
.github/PULL_REQUEST_TEMPLATE.md:
##########
@@ -0,0 +1,27 @@
+Thank you for contributing to Apache OpenNLP.
+
+In order to streamline the review of the contribution we ask you
+to ensure the following steps have been taken:
+
+### For all changes:
+- [ ] Is there a JIRA ticket associated with this PR? Is it referenced 
+     in the commit message?
+
+- [ ] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
+
+- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
+
+- [ ] Is your initial contribution a single, squashed commit?
+
+### For code changes:
+- [ ] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp folder?

Review Comment:
   s/opennlp/opennlp-sandbox? Maybe use backticks for `mvn clean install`



##########
.github/CONTRIBUTING.md:
##########
@@ -0,0 +1,11 @@
+# How to contribute to Apache OpenNLP
+
+Thank you for your intention to contribute to the Apache OpenNLP project. As an open-source community, we highly appreciate external contributions to our project.
+
+To make the process smooth for the project *committers* (those who review and accept changes) and *contributors* (those who propose new changes via pull requests), there are a few rules to follow.
+
+## Contribution Guidelines
+
+Please check out the [How to get involved](http://opennlp.apache.org/get-involved.html) to understand how contributions are made. 

Review Comment:
   s/http/https? The link below too.



##########
.github/PULL_REQUEST_TEMPLATE.md:
##########
@@ -0,0 +1,27 @@
+Thank you for contributing to Apache OpenNLP.
+
+In order to streamline the review of the contribution we ask you
+to ensure the following steps have been taken:
+
+### For all changes:
+- [ ] Is there a JIRA ticket associated with this PR? Is it referenced 
+     in the commit message?
+
+- [ ] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
+
+- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
+
+- [ ] Is your initial contribution a single, squashed commit?
+
+### For code changes:
+- [ ] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp folder?
+- [ ] Have you written or updated unit tests to verify your changes?
+- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
+- [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file in opennlp folder?

Review Comment:
   opennlp-sandbox? Below this line as well.



##########
pom.xml:
##########
@@ -0,0 +1,473 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+   Licensed to the Apache Software Foundation (ASF) under one
+   or more contributor license agreements.  See the NOTICE file
+   distributed with this work for additional information
+   regarding copyright ownership.  The ASF licenses this file
+   to you under the Apache License, Version 2.0 (the
+   "License"); you may not use this file except in compliance
+   with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing,
+   software distributed under the License is distributed on an
+   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+   KIND, either express or implied.  See the License for the
+   specific language governing permissions and limitations
+   under the License.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.apache</groupId>
+        <artifactId>apache</artifactId>
+        <version>29</version>
+        <relativePath />
+    </parent>
+
+    <groupId>org.apache.opennlp</groupId>
+    <artifactId>opennlp-sandbox</artifactId>
+    <version>2.1.1-SNAPSHOT</version>
+    <packaging>pom</packaging>
+
+    <name>Apache OpenNLP Sandbox</name>
+
+    <scm>
+        <connection>scm:git:https://github.com/apache/opennlp-sandbox.git</connection>
+        <developerConnection>scm:git:git@github.com:apache/opennlp-sandbox.git</developerConnection>
+        <url>https://github.com/apache/opennlp-sandbox.git</url>
+        <tag>HEAD</tag>
+    </scm>
+
+    <repositories>
+        <repository>
+            <id>apache.snapshots</id>
+            <name>Apache Snapshot Repository</name>
+            <url>https://repository.apache.org/snapshots</url>
+            <snapshots>
+                <enabled>true</enabled>
+            </snapshots>
+        </repository>
+    </repositories>
+
+    <mailingLists>
+        <mailingList>
+            <name>Apache OpenNLP Users</name>
+            <subscribe>users-subscribe@opennlp.apache.org</subscribe>
+            <unsubscribe>users-unsubscribe@opennlp.apache.org</unsubscribe>
+            <post>users@opennlp.apache.org</post>
+            <archive>http://mail-archives.apache.org/mod_mbox/opennlp-users/</archive>
+        </mailingList>
+
+        <mailingList>
+            <name>Apache OpenNLP Developers</name>
+            <subscribe>dev-subscribe@opennlp.apache.org</subscribe>
+            <unsubscribe>dev-unsubscribe@opennlp.apache.org</unsubscribe>
+            <post>dev@opennlp.apache.org</post>
+            <archive>http://mail-archives.apache.org/mod_mbox/opennlp-dev/</archive>
+        </mailingList>
+
+        <mailingList>
+            <name>Apache OpenNLP Commits</name>
+            <subscribe>commits-subscribe@opennlp.apache.org</subscribe>
+            <unsubscribe>commits-unsubscribe@opennlp.apache.org</unsubscribe>
+            <archive>http://mail-archives.apache.org/mod_mbox/opennlp-commits/</archive>
+        </mailingList>
+
+        <mailingList>
+            <name>Apache OpenNLP Issues</name>
+            <subscribe>issues-subscribe@opennlp.apache.org</subscribe>
+            <unsubscribe>issues-unsubscribe@opennlp.apache.org</unsubscribe>
+            <archive>http://mail-archives.apache.org/mod_mbox/opennlp-issues/</archive>
+        </mailingList>
+    </mailingLists>
+
+    <issueManagement>
+        <system>jira</system>
+        <url>https://issues.apache.org/jira/browse/OPENNLP</url>
+    </issueManagement>
+
+    <modules>
+        <module>caseditor-corpus-server-plugin</module>
+        <module>caseditor-opennlp-plugin</module>
+        <module>corpus-server</module>
+        <module>mahout-addon</module>
+        <module>mallet-addon</module>
+        <module>modelbuilder-addon</module>
+        <module>nlp-utils</module>
+        <module>opennlp-coref</module>
+        <module>opennlp-similarity</module>
+        <module>opennlp-wsd</module>
+        <module>tf-ner-poc</module>
+        <module>tagging-server</module>
+        <module>wikinews-importer</module>
+    </modules>
+
+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>org.junit.jupiter</groupId>
+                <artifactId>junit-jupiter-api</artifactId>
+                <version>${junit.version}</version>
+                <scope>test</scope>
+            </dependency>
+
+            <dependency>
+                <groupId>org.junit.jupiter</groupId>
+                <artifactId>junit-jupiter-engine</artifactId>
+                <version>${junit.version}</version>
+                <scope>test</scope>
+            </dependency>
+
+            <dependency>
+                <groupId>org.junit.jupiter</groupId>
+                <artifactId>junit-jupiter-params</artifactId>
+                <version>${junit.version}</version>
+                <scope>test</scope>
+            </dependency>
+
+            <dependency>
+                <artifactId>opennlp-tools</artifactId>
+                <groupId>${project.groupId}</groupId>
+                <version>${opennlp.tools.version}</version>
+            </dependency>
+
+            <dependency>
+                <artifactId>opennlp-tools</artifactId>
+                <groupId>${project.groupId}</groupId>
+                <version>${project.version}</version>
+                <type>test-jar</type>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+
+    <properties>
+        <!-- Build Properties -->
+        <java.version>11</java.version>
+        <maven.version>3.3.9</maven.version>
+        <maven.compiler.source>${java.version}</maven.compiler.source>
+        <maven.compiler.target>${java.version}</maven.compiler.target>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        
+        <opennlp.tools.version>2.1.0</opennlp.tools.version>
+        <commons.io.version>2.6</commons.io.version>
+        <uimaj.version>3.3.1</uimaj.version>
+
+        <junit.version>5.9.1</junit.version>
+        
+        <enforcer.plugin.version>3.0.0-M3</enforcer.plugin.version>
+        <checkstyle.plugin.version>3.2.0</checkstyle.plugin.version>
+        <opennlp.forkCount>1.0C</opennlp.forkCount>
+        <coveralls.maven.plugin>4.3.0</coveralls.maven.plugin>
+        <jacoco.maven.plugin>0.7.9</jacoco.maven.plugin>
+        <maven.surefire.plugin>2.22.2</maven.surefire.plugin>
+        <maven.failsafe.plugin>2.22.2</maven.failsafe.plugin>
+        <mockito.version>3.9.0</mockito.version>
+    </properties>
+
+    <build>
+        <pluginManagement>
+            <plugins>
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-release-plugin</artifactId>
+                    <configuration>
+                        <useReleaseProfile>false</useReleaseProfile>
+                        <goals>deploy</goals>
+                        <arguments>-Papache-release</arguments>
+                        <mavenExecutorId>forked-path</mavenExecutorId>
+                    </configuration>
+                </plugin>
+
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-assembly-plugin</artifactId>
+                    <version>3.2.0</version>
+                </plugin>
+
+                <plugin>
+                    <groupId>org.apache.felix</groupId>
+                    <artifactId>maven-bundle-plugin</artifactId>
+                    <version>5.1.4</version>
+                </plugin>
+                <!--
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-checkstyle-plugin</artifactId>
+                    <version>${checkstyle.plugin.version}</version>
+                    <dependencies>
+                        <dependency>
+                            <groupId>com.puppycrawl.tools</groupId>
+                            <artifactId>checkstyle</artifactId>
+                            <version>10.6.0</version>
+                        </dependency>
+                    </dependencies>
+                    <executions>
+                        <execution>
+                            <id>validate</id>
+                            <phase>validate</phase>
+                            <configuration>
+                                <configLocation>checkstyle.xml</configLocation>
+                                <consoleOutput>true</consoleOutput>
+                                <includeTestSourceDirectory>true</includeTestSourceDirectory>
+                                <testSourceDirectories>${project.basedir}/src/test/java</testSourceDirectories>
+                                <violationSeverity>error</violationSeverity>
+                                <failOnViolation>true</failOnViolation>
+                            </configuration>
+                            <goals>
+                                <goal>check</goal>
+                            </goals>
+                        </execution>
+                    </executions>
+                </plugin>
+                -->
+                
+                <!-- Coverage analysis for tests -->
+                <plugin>
+                    <groupId>org.jacoco</groupId>
+                    <artifactId>jacoco-maven-plugin</artifactId>
+                    <version>${jacoco.maven.plugin}</version>
+                    <configuration>
+                        <excludes>
+                            <exclude>**/stemmer/*</exclude>
+                            <exclude>**/stemmer/snowball/*</exclude>
+                        </excludes>
+                    </configuration>
+                    <executions>
+                        <execution>
+                            <id>jacoco-prepare-agent</id>
+                            <goals>
+                                <goal>prepare-agent</goal>
+                            </goals>
+                        </execution>
+                        <execution>
+                            <id>jacoco-prepare-agent-integration</id>
+                            <goals>
+                                <goal>prepare-agent-integration</goal>
+                            </goals>
+                        </execution>
+                        <execution>
+                            <id>jacoco-report</id>
+                            <phase>verify</phase>
+                            <goals>
+                                <goal>report</goal>
+                            </goals>
+                        </execution>
+                    </executions>
+                </plugin>
+
+                <!-- Report jacoco coverage to coveralls.io -->
+                <plugin>
+                    <groupId>org.eluder.coveralls</groupId>
+                    <artifactId>coveralls-maven-plugin</artifactId>
+                    <version>${coveralls.maven.plugin}</version>
+                </plugin>
+
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-surefire-plugin</artifactId>
+                    <version>${maven.surefire.plugin}</version>
+                    <configuration>
+                        <argLine>-Xmx2048m</argLine>
+                        <forkCount>${opennlp.forkCount}</forkCount>
+                        <failIfNoSpecifiedTests>false</failIfNoSpecifiedTests>
+                        <excludes>
+                            <exclude>**/stemmer/*</exclude>
+                            <exclude>**/stemmer/snowball/*</exclude>
+                            <exclude>**/*IT.java</exclude>
+                        </excludes>
+                    </configuration>
+                </plugin>
+
+                <plugin>
+                    <groupId>org.apache.maven.plugins</groupId>
+                    <artifactId>maven-failsafe-plugin</artifactId>
+                    <version>${maven.failsafe.plugin}</version>
+                    <executions>
+                        <execution>
+                            <id>integration-test</id>
+                            <goals>
+                                <goal>integration-test</goal>
+                                <goal>verify</goal>
+                            </goals>
+                        </execution>
+                    </executions>
+                    <configuration>
+                        <excludes>
+                            <exclude>**/*Test.java</exclude>
+                        </excludes>
+                        <includes>
+                            <include>**/*IT.java</include>
+                        </includes>
+                    </configuration>
+                </plugin>
+
+                <plugin>
+                    <groupId>de.thetaphi</groupId>
+                    <artifactId>forbiddenapis</artifactId>
+                    <version>2.7</version>
+                    <configuration>
+                        <failOnUnsupportedJava>false</failOnUnsupportedJava>
+                        <bundledSignatures>
+                            <bundledSignature>jdk-deprecated</bundledSignature>
+                            <bundledSignature>jdk-non-portable</bundledSignature>
+                        </bundledSignatures>
+                    </configuration>
+                    <executions>
+                        <execution>
+                            <phase>validate</phase>
+                            <goals>
+                                <goal>check</goal>
+                                <goal>testCheck</goal>
+                            </goals>
+                        </execution>
+                    </executions>
+                </plugin>
+            </plugins>
+        </pluginManagement>
+
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <version>3.10.1</version>
+                <configuration>
+                    <release>${java.version}</release>
+                    <compilerArgument>-Xlint</compilerArgument>
+                </configuration>
+            </plugin>
+            <!--
+            <plugin>
+                <groupId>org.apache.rat</groupId>
+                <artifactId>apache-rat-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>default-cli</id>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                        <phase>verify</phase>
+                        <configuration>
+                            <excludes>
+                                <exclude>release.properties</exclude>
+                            </excludes>
+                            <numUnapprovedLicenses>1000000</numUnapprovedLicenses>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
+            -->

Review Comment:
   Do we need `TODO`s for the commented code in this file?



##########
.github/PULL_REQUEST_TEMPLATE.md:
##########
@@ -0,0 +1,27 @@
+Thank you for contributing to Apache OpenNLP.
+
+In order to streamline the review of the contribution we ask you
+to ensure the following steps have been taken:
+
+### For all changes:
+- [ ] Is there a JIRA ticket associated with this PR? Is it referenced 
+     in the commit message?
+
+- [ ] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
+
+- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
+
+- [ ] Is your initial contribution a single, squashed commit?
+
+### For code changes:
+- [ ] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp folder?
+- [ ] Have you written or updated unit tests to verify your changes?
+- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 

Review Comment:
   s/http/https to save 1 redirect for users/browsers 



##########
mahout-addon/pom.xml:
##########
@@ -21,15 +21,12 @@
 
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
 	<modelVersion>4.0.0</modelVersion>
-
 	<parent>
-		<groupId>org.apache</groupId>
-		<artifactId>apache</artifactId>
-		<!-- TODO OPENNLP-1452 once this is resolved, move to 29 as well. -->
-		<version>18</version>
-		<relativePath />
+		<groupId>org.apache.opennlp</groupId>
+		<artifactId>opennlp-sandbox</artifactId>
+		<version>2.1.1-SNAPSHOT</version>
 	</parent>
-
+    

Review Comment:
   Extra spaces added?



##########
.gitattributes:
##########
@@ -0,0 +1,48 @@
+# Handle line endings automatically for files detected as text
+# and leave all files detected as binary untouched.
+* text=auto
+
+#
+# The above will handle all files NOT found below
+#
+# These files are text and should be normalized (Convert crlf => lf)
+*.adoc          text    eol=lf
+*.html          text    eol=lf
+*.java          text    eol=lf
+*.jspf          text    eol=lf
+*.md            text    eol=lf
+*.properties    text    eol=lf
+*.sh            text    eol=lf
+*.txt           text    eol=lf
+*.xml           text    eol=lf
+*.xsd           text    eol=lf
+*.xsl           text    eol=lf
+*.yml           text    eol=lf
+
+LICENSE         text    eol=lf
+NOTICE          text    eol=lf
+
+# These files are binary and should be left untouched
+# (binary is a macro for -text -diff)
+*.class         binary
+*.dll           binary
+*.ear           binary
+*.gif           binary
+*.ico           binary
+*.jar           binary
+*.jpg           binary
+*.jpeg          binary
+*.png           binary
+*.ser           binary
+*.so            binary
+*.war           binary
+*.zip           binary
+*.exe           binary
+*.gz            binary
+
+#Windows
+*.bat text eol=crlf
+*.cmd text eol=crlf
+
+#Unix/Linux
+*.sh text eol=lf

Review Comment:
   Some files have the missing newline... I'm fine both ways, but sometimes these pop up in reviews when contributors have their editors automatically adding the newline.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org