[GitHub] commons-lang pull request #254: Add checkstyle to the CI
GitHub user mureinik opened a pull request: https://github.com/apache/commons-lang/pull/254 Add checkstyle to the CI The Apache Commons Lang project uses checktyle to enforce its coding style, but this is only done as part of the reporting phase, when the site is generated. Thus, using checkstyle becomes somewhat of an opt-in procedure, depending on the vigilance of the developers to check the report, instead of being an actual enforcement against violating the codestyle, as evident by the fact that the current checkstyle report contains several hundred(!) such violations. This PR contains a series of patches to fix the existing checkstyle violations and a final patch that adds checkstyle to Travis CI, making it a gating check instead of an opt-in practice. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mureinik/commons-lang checkstyle Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-lang/pull/254.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #254 commit 13ebd5a955787ca91990a713c8cdf482aa72ca2a Author: Allon Mureinik Date: 2017-03-05T10:05:23Z Apply JavaDoc checkstyle only to public methods The defacto style of the project requires Javadoc for public methods only, but the checkstyle Javadoc check defaults to requiring them even for private methods, generating 46 errors. This patch sets the checkstyle Javadoc check's scope to public to clean up the checkstyle report so it can be enabled in the CI. If we wish to reset the check to a laxer scope, the aforementioned errors should be fixed first. commit c1c21cd9a7d1888f054813a50ab6c6a3b0517292 Author: Allon Mureinik Date: 2017-03-11T07:52:41Z Remove unused SystemUtils import from ExceptionUtils commit e14660f1c86acf8c2432593d9ad1a51108c2e83c Author: Allon Mureinik Date: 2017-03-11T07:54:27Z Replace tabs with spaces in DateUtils commit a7d7e37d091bf1ba3674c6fc617466b284aaac71 Author: Allon Mureinik Date: 2017-03-11T08:13:44Z Upgrade maven-checkstyle-plugin to 2.17 This patch upgrades maven-checkstyle-plugin to the latest available version, 2.17. This is done in order to consume a fix for checkstyle wrongfully reporting an error if the @return javadoc tag was used in an annotation type, as it is in Guarded (line 36). Note that checkstyle has removed the RedundantThrows check (see discussion at https://github.com/checkstyle/checkstyle/issues/473), so it was removed from the project's checkstyle.xml configuration. commit c407e34e7ecfde0e72bbbed1e5e8106792140d64 Author: Allon Mureinik Date: 2017-03-11T08:49:22Z org.apache.commons.lang3.concurrent.annotation package-info Added package-info.java to the org.apache.commons.lang3.concurrent.annotation package to solve a checkstyle violation. commit 063e538595897a481f93d0a926a09081777eae4a Author: Allon Mureinik Date: 2017-03-11T09:14:16Z Add checkstyle to Travis CI Currently, checkstyle is only run as part of the reporting phase, and it's up to the developer to check the report manually. This patch adds the checkstyle configuration to the build plugins so it can be used to check the code (as opposed to just generate a report of the failures) and adds it to Travis CI's configuration so every new patch will be automatically checked against it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #254: Add checkstyle to the CI
Github user kinow commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/254#discussion_r105530182 --- Diff: checkstyle.xml --- @@ -36,11 +36,9 @@ limitations under the License. - --- End diff -- Any reason for removing this one? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #254: Add checkstyle to the CI
Github user kinow commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/254#discussion_r105530165 --- Diff: src/main/java/org/apache/commons/lang3/concurrent/annotation/package-info.java --- @@ -0,0 +1,22 @@ +/* + * 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. + */ +/** + * Provides annotations to document classes' thraed safety --- End diff -- typo s/thraed/thread --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #254: Add checkstyle to the CI
Github user mureinik commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/254#discussion_r105530325 --- Diff: src/main/java/org/apache/commons/lang3/concurrent/annotation/package-info.java --- @@ -0,0 +1,22 @@ +/* + * 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. + */ +/** + * Provides annotations to document classes' thraed safety --- End diff -- Arg :-( Will fix, thanks for noticing! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #254: Add checkstyle to the CI
Github user mureinik commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/254#discussion_r105530384 --- Diff: checkstyle.xml --- @@ -36,11 +36,9 @@ limitations under the License. - --- End diff -- Yes. I explained this in the relevant commit (a7d7e37d091bf1ba3674c6fc617466b284aaac71) , but perhaps I should add the explanation to the PR's comment too, to make it more obvious. New checkstyle versions removed this check, so we can no longer use it. Since checkstyle is not type-aware, there's really no way of performing this check properly - see https://github.com/checkstyle/checkstyle/issues/473 for the discussion about this removal. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #254: Add checkstyle to the CI
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/254 Latest push fixes the typo (thraed->thread) as per @kinow's review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang pull request #254: Add checkstyle to the CI
Github user kinow commented on a diff in the pull request: https://github.com/apache/commons-lang/pull/254#discussion_r105530594 --- Diff: checkstyle.xml --- @@ -36,11 +36,9 @@ limitations under the License. - --- End diff -- :+1: thanks for the explanation and the links @mureinik --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Commented] (NET-584) Error with org.apache.commons.net.ftp.FTPClient
[ https://issues.apache.org/jira/browse/NET-584?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15906152#comment-15906152 ] Sebb commented on NET-584: -- The NOOPs are only (potentially) needed for long running data transfers which pass through a router. They should not be needed for talking directly to an FTP server, because the server knows which control channels are associated with which data channels. (If it does not, it is badly broken!) Some routers are clever enough to do the same; some just see an idle channel and assume it has been abandoned. These latter need NOOPs to try and prevent disconnection. FTPClient only sends the NOOPs if the wait time is > 0 and the command is one of the retrieve/store operations that don't involve application interaction. In other cases where the control channel may be idle for long periods, it is up to the app to send NOOPs. FTP servers that can support parallel control channel traffic will respond to the NOOPs during the data transfer. If not, then the replies will likely be sent when the transfer completes and the server returns to processing the control channel. FTPClient keeps a count of the number of unanswered NOOPs and tries to drain these at the end of the transfer. If any requests or responses got lost, then of course we get a timeout, which is what I think we are seeing. I guess the longer the data transfer, the more likely it is that they are lost. This may explain the behaviour you are seeing with bigger files. Note: the SNAPSHOT code now assumes that a timeout during cleanup means that there are no more replies left and does not throw an exception (it writes to stderr instead). I think it is correct to ignore the timeout here; however writing to stderr is no use for production code. Need to either ignore the timeout or find a way to let the app know (or a way for the app to find out). Meanwhile it would be very helpful if you were able to try the updated code. > Error with org.apache.commons.net.ftp.FTPClient > > > Key: NET-584 > URL: https://issues.apache.org/jira/browse/NET-584 > Project: Commons Net > Issue Type: Bug > Components: FTP >Reporter: Kazantsev Andrey Sergeevich > > I have a question about using library commons-net-3.4.jar > Question is about org.apache.commons.net.ftp.FTPClient method > setControlKeepAliveTimeout. > Read about using it on: > https://commons.apache.org/proper/commons-net/apidocs/org/apache/commons/net/ftp/FTPClient.html > When I use it in my code I get this error: > {code} > java.net.SocketTimeoutException: Read timed out > at java.net.SocketInputStream.socketRead0(Native Method) > at java.net.SocketInputStream.read(SocketInputStream.java:163) > at java.net.SocketInputStream.read(SocketInputStream.java:133) > at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:322) > at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:364) > at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:210) > at java.io.InputStreamReader.read(InputStreamReader.java:205) > at java.io.BufferedReader.fill(BufferedReader.java:165) > at java.io.BufferedReader.read(BufferedReader.java:186) > at > org.apache.commons.net.io.CRLFLineReader.readLine(CRLFLineReader.java:58) > at org.apache.commons.net.ftp.FTP.__getReply(FTP.java:313) > at org.apache.commons.net.ftp.FTP.__getReplyNoReport(FTP.java:303) > at org.apache.commons.net.ftp.FTPClient$CSL.cleanUp(FTPClient.java:3838) > at org.apache.commons.net.ftp.FTPClient._storeFile(FTPClient.java:695) > at org.apache.commons.net.ftp.FTPClient.__storeFile(FTPClient.java:643) > at org.apache.commons.net.ftp.FTPClient.storeFile(FTPClient.java:2033) > at ru.mdm.File.Transfer.FTP.PutRemoteFileBinary(FTP.java:192) > at > ru.mdm.File.Transfer.TimeLimit.Thread.Protocol.PutRemoteFileBinaryThread.actionsToExecute(PutRemoteFileBinaryThread.java:23) > at > ru.mdm.File.Transfer.TimeLimit.OperationThread.run(OperationThread.java:60) > {code} > Without enabling this option all works fine. > Here is the code: > {code} > package ru.mdm.File.Transfer; > import bin.ru.osa.common.utils.*; > import java.util.List; > import java.io.*; > import com.ibm.broker.javacompute.MbJavaComputeNode; > import com.ibm.broker.plugin.*; > import org.apache.commons.net.ftp.*; > import org.apache.commons.net.*; > import ru.mdm.File.Transfer.Options.OptionsXMLProcessor; > public class FTP implements Protocol > { > > FTPClient client = new FTPClient(); > > OptionsXMLProcessor optionsXMLProcessor; > > > boolean st; > String LastMessage = new String(); > > boolean ignoreErrors = false; > > > public FTP() > { >
[jira] [Resolved] (NET-622) SubnetInfo#getCidrSignature - improve functions
[ https://issues.apache.org/jira/browse/NET-622?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sebb resolved NET-622. -- Resolution: Fixed Fix Version/s: 3.7 Thanks, fixed: URL: http://svn.apache.org/viewvc?rev=1786507&view=rev Log: NET-624 SubnetInfo#getCidrSignature - improve functions Modified: commons/proper/net/trunk/src/changes/changes.xml commons/proper/net/trunk/src/main/java/org/apache/commons/net/util/SubnetUtils.java > SubnetInfo#getCidrSignature - improve functions > --- > > Key: NET-622 > URL: https://issues.apache.org/jira/browse/NET-622 > Project: Commons Net > Issue Type: Improvement >Reporter: Makoto Sakaguchi >Priority: Minor > Fix For: 3.7 > > > The processing part of "netmask" is converted an integer to a string to an > integer. > {code} > /* SubnetUtils.SubnetInfo#getCidrSignature() */ > // a binary mask address > int netmask = 0b; > // convert to a dotted decimal address > String maskStr = format(toArray(netmask)); // maskStr: "255.0.0.0" > /* SubnetUtils#toCidrNotation() */ > // counting the 1-bit population > int cidr = pop(toInteger(maskStr)); // cidr: 8 > {code} > So, I suggest to create CIDR from the netmask variable. > https://github.com/apache/commons-net/pull/10 -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Created] (NET-625) SubnetUtils - improve construction
Sebb created NET-625: Summary: SubnetUtils - improve construction Key: NET-625 URL: https://issues.apache.org/jira/browse/NET-625 Project: Commons Net Issue Type: Improvement Affects Versions: 3.6 Reporter: Sebb The constructors for CIDR format and (address, mask) format both use the same calculate() method. This assumes the input is in CIDR format, which means that the constructor which handles (address, mask) format has to convert the strings to binary and then recreate the CIDR format. This is wasteful. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Resolved] (NET-625) SubnetUtils - improve construction
[ https://issues.apache.org/jira/browse/NET-625?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Sebb resolved NET-625. -- Resolution: Fixed Fix Version/s: 3.7 URL: http://svn.apache.org/viewvc?rev=1786511&view=rev Log: NET-625 SubnetUtils - improve construction Also make most fields final Modified: commons/proper/net/trunk/src/changes/changes.xml commons/proper/net/trunk/src/main/java/org/apache/commons/net/util/SubnetUtils.java > SubnetUtils - improve construction > -- > > Key: NET-625 > URL: https://issues.apache.org/jira/browse/NET-625 > Project: Commons Net > Issue Type: Improvement >Affects Versions: 3.6 >Reporter: Sebb > Fix For: 3.7 > > > The constructors for CIDR format and (address, mask) format both use the same > calculate() method. This assumes the input is in CIDR format, which means > that the constructor which handles (address, mask) format has to convert the > strings to binary and then recreate the CIDR format. > This is wasteful. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[GitHub] commons-lang issue #254: Add checkstyle to the CI
Github user mureinik commented on the issue: https://github.com/apache/commons-lang/pull/254 CI seems to have been stuck for several hours. What am I missing here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #254: Add checkstyle to the CI
Github user PascalSchumacher commented on the issue: https://github.com/apache/commons-lang/pull/254 I think that is normal lately. Seems like a lot of apache projects enabled travis recently and now apaches travis-ci capacity can not handle the load. Sometimes it takes a day until the build runs. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #254: Add checkstyle to the CI
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/254 [![Coverage Status](https://coveralls.io/builds/10546889/badge)](https://coveralls.io/builds/10546889) Coverage remained the same at 94.529% when pulling **417c2931cef184843a58c85b7bdddc6210494e3a on mureinik:checkstyle** into **ee19f8247c4bc0335ad3efe212dc0bdce97e2034 on apache:master**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] commons-lang issue #254: Add checkstyle to the CI
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/254 [![Coverage Status](https://coveralls.io/builds/10546929/badge)](https://coveralls.io/builds/10546929) Coverage remained the same at 94.529% when pulling **417c2931cef184843a58c85b7bdddc6210494e3a on mureinik:checkstyle** into **ee19f8247c4bc0335ad3efe212dc0bdce97e2034 on apache:master**. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[jira] [Created] (TEXT-72) Similar to LANG-1025
Rob Tompkins created TEXT-72: Summary: Similar to LANG-1025 Key: TEXT-72 URL: https://issues.apache.org/jira/browse/TEXT-72 Project: Commons Text Issue Type: Bug Affects Versions: 1.0 Reporter: Rob Tompkins Fix For: 1.x clirr seems to be failing to build properly with java 1.8.0_121 and maven 3.3.9. Analogous to https://issues.apache.org/jira/browse/LANG-1025, we are upgrading to clirr 2.8. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Updated] (TEXT-72) Similar to LANG-1025, clirr fails site build.
[ https://issues.apache.org/jira/browse/TEXT-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Rob Tompkins updated TEXT-72: - Summary: Similar to LANG-1025, clirr fails site build. (was: Similar to LANG-1025) > Similar to LANG-1025, clirr fails site build. > - > > Key: TEXT-72 > URL: https://issues.apache.org/jira/browse/TEXT-72 > Project: Commons Text > Issue Type: Bug >Affects Versions: 1.0 >Reporter: Rob Tompkins > Fix For: 1.x > > > clirr seems to be failing to build properly with java 1.8.0_121 and maven > 3.3.9. Analogous to https://issues.apache.org/jira/browse/LANG-1025, we are > upgrading to clirr 2.8. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (TEXT-72) Similar to LANG-1025, clirr fails site build.
[ https://issues.apache.org/jira/browse/TEXT-72?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15906399#comment-15906399 ] Rob Tompkins commented on TEXT-72: -- https://git1-us-west.apache.org/repos/asf?p=commons-text.git;a=commit;h=94ca2ca0 > Similar to LANG-1025, clirr fails site build. > - > > Key: TEXT-72 > URL: https://issues.apache.org/jira/browse/TEXT-72 > Project: Commons Text > Issue Type: Bug >Affects Versions: 1.0 >Reporter: Rob Tompkins > Fix For: 1.x > > > clirr seems to be failing to build properly with java 1.8.0_121 and maven > 3.3.9. Analogous to https://issues.apache.org/jira/browse/LANG-1025, we are > upgrading to clirr 2.8. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Resolved] (TEXT-72) Similar to LANG-1025, clirr fails site build.
[ https://issues.apache.org/jira/browse/TEXT-72?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Rob Tompkins resolved TEXT-72. -- Resolution: Fixed Assignee: Rob Tompkins > Similar to LANG-1025, clirr fails site build. > - > > Key: TEXT-72 > URL: https://issues.apache.org/jira/browse/TEXT-72 > Project: Commons Text > Issue Type: Bug >Affects Versions: 1.0 >Reporter: Rob Tompkins >Assignee: Rob Tompkins > Fix For: 1.x > > > clirr seems to be failing to build properly with java 1.8.0_121 and maven > 3.3.9. Analogous to https://issues.apache.org/jira/browse/LANG-1025, we are > upgrading to clirr 2.8. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Created] (NET-626) SubnetUtils#SubnetUtils - improved comment
Makoto Sakaguchi created NET-626: Summary: SubnetUtils#SubnetUtils - improved comment Key: NET-626 URL: https://issues.apache.org/jira/browse/NET-626 Project: Commons Net Issue Type: Improvement Reporter: Makoto Sakaguchi Priority: Minor Describe of CIDR value was more clearly. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Updated] (NET-626) SubnetUtils#SubnetUtils - improved comment
[ https://issues.apache.org/jira/browse/NET-626?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Makoto Sakaguchi updated NET-626: - Description: Describe of CIDR value was more clearly. https://github.com/apache/commons-net/pull/16 was:Describe of CIDR value was more clearly. > SubnetUtils#SubnetUtils - improved comment > -- > > Key: NET-626 > URL: https://issues.apache.org/jira/browse/NET-626 > Project: Commons Net > Issue Type: Improvement >Reporter: Makoto Sakaguchi >Priority: Minor > > Describe of CIDR value was more clearly. > https://github.com/apache/commons-net/pull/16 -- This message was sent by Atlassian JIRA (v6.3.15#6346)