[GitHub] [commons-lang] EACUAMBA closed pull request #889: Added method to convert Object to String that support null values.
EACUAMBA closed pull request #889: Added method to convert Object to String that support null values. URL: https://github.com/apache/commons-lang/pull/889 -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-lang] EACUAMBA commented on a diff in pull request #889: Added method to convert Object to String that support null values.
EACUAMBA commented on code in PR #889: URL: https://github.com/apache/commons-lang/pull/889#discussion_r862528440 ## src/main/java/org/apache/commons/lang3/StringUtils.java: ## @@ -1588,6 +1588,59 @@ public static String defaultString(final String str, final String nullDefault) { return Objects.toString(str, nullDefault); } +/** + * Returns either the passed in String, + * or if the String is {@code null}, an empty String (""). + * + * + * StringUtils.defaultString(null) = "" + * StringUtils.defaultString("")= "" + * StringUtils.defaultString("bat") = "bat" + * StringUtils.defaultString(1) = "1" + * StringUtils.defaultString(Integer.MIN_VALUE) = "-2147483648" + * + * + * @see Objects#toString(Object) + * @see String#valueOf(Object) + * @param str the Object to check, may be null + * @return the passed in String, or the empty String if it + * was {@code null} + */ +public static String defaultString(final Object object) { +if(object == null) +return ""; + +return Objects.toString(object); +} + +/** + * Returns either the given String, or if the String is + * {@code null}, {@code nullDefault}. + * + * + * StringUtils.defaultString(null, "NULL") = "NULL" + * StringUtils.defaultString("", "NULL")= "" + * StringUtils.defaultString("bat", "NULL") = "bat" + * StringUtils.defaultString(1, "20") = "1" + * StringUtils.defaultString(Integer.MIN_VALUE, "200") = "-2147483648" + * + * + * @see Objects#toString(Object, String) + * @see String#valueOf(Object) + * @param str the String to check, may be null + * @param nullDefault the default String to return + * if the input is {@code null}, may be null + * @return the passed in String, or the default if it was {@code null} + * @deprecated Use {@link Objects#toString(Object, String)} + */ +@Deprecated +public static String defaultString(final Object object, final String nullDefault) { +if(object == null) +return defaultString(nullDefault); + +return Objects.toString(object); Review Comment: Hi @kinow, You are right, the toString method in Objects can work too. Thanks. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-lang] kinow commented on a diff in pull request #889: Added method to convert Object to String that support null values.
kinow commented on code in PR #889: URL: https://github.com/apache/commons-lang/pull/889#discussion_r862522289 ## src/main/java/org/apache/commons/lang3/StringUtils.java: ## @@ -1588,6 +1588,59 @@ public static String defaultString(final String str, final String nullDefault) { return Objects.toString(str, nullDefault); } +/** + * Returns either the passed in String, + * or if the String is {@code null}, an empty String (""). + * + * + * StringUtils.defaultString(null) = "" + * StringUtils.defaultString("")= "" + * StringUtils.defaultString("bat") = "bat" + * StringUtils.defaultString(1) = "1" + * StringUtils.defaultString(Integer.MIN_VALUE) = "-2147483648" + * + * + * @see Objects#toString(Object) + * @see String#valueOf(Object) + * @param str the Object to check, may be null + * @return the passed in String, or the empty String if it + * was {@code null} + */ +public static String defaultString(final Object object) { +if(object == null) +return ""; + +return Objects.toString(object); +} + +/** + * Returns either the given String, or if the String is + * {@code null}, {@code nullDefault}. + * + * + * StringUtils.defaultString(null, "NULL") = "NULL" + * StringUtils.defaultString("", "NULL")= "" + * StringUtils.defaultString("bat", "NULL") = "bat" + * StringUtils.defaultString(1, "20") = "1" + * StringUtils.defaultString(Integer.MIN_VALUE, "200") = "-2147483648" + * + * + * @see Objects#toString(Object, String) + * @see String#valueOf(Object) + * @param str the String to check, may be null + * @param nullDefault the default String to return + * if the input is {@code null}, may be null + * @return the passed in String, or the default if it was {@code null} + * @deprecated Use {@link Objects#toString(Object, String)} + */ +@Deprecated +public static String defaultString(final Object object, final String nullDefault) { +if(object == null) +return defaultString(nullDefault); + +return Objects.toString(object); Review Comment: Hi @EACUAMBA , I think instead of this new method, users could just use the Java 8+ `Objects.toString(Object o, String nullDefault)` - https://docs.oracle.com/javase/8/docs/api/java/util/Objects.html#toString-java.lang.Object-java.lang.String- -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Work logged] (COLLECTIONS-811) Consider integration Guava testlib tests
[ https://issues.apache.org/jira/browse/COLLECTIONS-811?focusedWorklogId=764804&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-764804 ] ASF GitHub Bot logged work on COLLECTIONS-811: -- Author: ASF GitHub Bot Created on: 01/May/22 21:04 Start Date: 01/May/22 21:04 Worklog Time Spent: 10m Work Description: ben-manes commented on code in PR #301: URL: https://github.com/apache/commons-collections/pull/301#discussion_r862521177 ## src/test/java/org/apache/commons/collections4/GuavaTestlibTest.java: ## @@ -0,0 +1,76 @@ +/* + * 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. + */ + +package org.apache.commons.collections4; + +import java.util.Map; +import java.util.function.Supplier; + +import org.apache.commons.collections4.map.HashedMap; +import org.apache.commons.collections4.map.LRUMap; +import org.apache.commons.collections4.map.LinkedMap; +import org.apache.commons.collections4.map.ReferenceMap; + +import com.google.common.collect.testing.MapTestSuiteBuilder; +import com.google.common.collect.testing.TestStringMapGenerator; +import com.google.common.collect.testing.features.CollectionFeature; +import com.google.common.collect.testing.features.CollectionSize; +import com.google.common.collect.testing.features.MapFeature; + +import junit.framework.Test; +import junit.framework.TestCase; +import junit.framework.TestSuite; + +/** + * This test uses Google's Guava Testlib testing libraries to validate the + * contract of collection classes in Commons Collections. This was introduced + * after COLLECTIONS-802, where the issue reported was found with Testlib, + * with thanks to Ben Manes. + * + * @since 4.5.0 + * @see https://github.com/google/guava/tree/master/guava-testlib";>https://github.com/google/guava/tree/master/guava-testlib + * @see https://issues.apache.org/jira/browse/COLLECTIONS-802";>https://issues.apache.org/jira/browse/COLLECTIONS-802 + */ +public final class GuavaTestlibTest extends TestCase { + +public static Test suite() { Review Comment: At least in the case of `GrowthList` some of the failures are expected since it modified the `List` contract, ```java * Decorates another List to make it seamlessly grow when * indices larger than the list size are used on add and set, * avoiding most IndexOutOfBoundsExceptions. ``` Technically a violation of the contract, but expected. ```java * @throws IndexOutOfBoundsException if the index is out of range * ({@code index < 0 || index > size()}) */ void add(int index, E element); ``` You can use `.suppressing(Method...)` to disable those cases. Issue Time Tracking --- Worklog Id: (was: 764804) Time Spent: 2h 10m (was: 2h) > Consider integration Guava testlib tests > > > Key: COLLECTIONS-811 > URL: https://issues.apache.org/jira/browse/COLLECTIONS-811 > Project: Commons Collections > Issue Type: Test >Affects Versions: 4.4 >Reporter: Bruno P. Kinoshita >Assignee: Bruno P. Kinoshita >Priority: Minor > Fix For: 4.5 > > Time Spent: 2h 10m > Remaining Estimate: 0h > > In COLLECTIONS-802 an issue reported was found with the help of Google Guava > testlib tests. > Maybe we could either have something similar (i.e. build ourselves?), use > Google Guava's testlib, or find another similar solution. From what I > understood, it uses a factory function to create an implementation of a > collection interface (e.g. Map) and then runs a series of functional tests > over the created object, failing tests if a contract is broken (e.g. > iterating a map doesn't leave the next-object as null, as it was the case of > the 802 issue). -- This message was sent by Atlassian Jira (v8.20.7#820007)
[GitHub] [commons-collections] ben-manes commented on a diff in pull request #301: [COLLECTIONS-811] Integrate Guava Testlib tests for Apache Commons Collections
ben-manes commented on code in PR #301: URL: https://github.com/apache/commons-collections/pull/301#discussion_r862521177 ## src/test/java/org/apache/commons/collections4/GuavaTestlibTest.java: ## @@ -0,0 +1,76 @@ +/* + * 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. + */ + +package org.apache.commons.collections4; + +import java.util.Map; +import java.util.function.Supplier; + +import org.apache.commons.collections4.map.HashedMap; +import org.apache.commons.collections4.map.LRUMap; +import org.apache.commons.collections4.map.LinkedMap; +import org.apache.commons.collections4.map.ReferenceMap; + +import com.google.common.collect.testing.MapTestSuiteBuilder; +import com.google.common.collect.testing.TestStringMapGenerator; +import com.google.common.collect.testing.features.CollectionFeature; +import com.google.common.collect.testing.features.CollectionSize; +import com.google.common.collect.testing.features.MapFeature; + +import junit.framework.Test; +import junit.framework.TestCase; +import junit.framework.TestSuite; + +/** + * This test uses Google's Guava Testlib testing libraries to validate the + * contract of collection classes in Commons Collections. This was introduced + * after COLLECTIONS-802, where the issue reported was found with Testlib, + * with thanks to Ben Manes. + * + * @since 4.5.0 + * @see https://github.com/google/guava/tree/master/guava-testlib";>https://github.com/google/guava/tree/master/guava-testlib + * @see https://issues.apache.org/jira/browse/COLLECTIONS-802";>https://issues.apache.org/jira/browse/COLLECTIONS-802 + */ +public final class GuavaTestlibTest extends TestCase { + +public static Test suite() { Review Comment: At least in the case of `GrowthList` some of the failures are expected since it modified the `List` contract, ```java * Decorates another List to make it seamlessly grow when * indices larger than the list size are used on add and set, * avoiding most IndexOutOfBoundsExceptions. ``` Technically a violation of the contract, but expected. ```java * @throws IndexOutOfBoundsException if the index is out of range * ({@code index < 0 || index > size()}) */ void add(int index, E element); ``` You can use `.suppressing(Method...)` to disable those cases. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-lang] EACUAMBA opened a new pull request, #889: Added method to convert Object to String that support null values.
EACUAMBA opened a new pull request, #889: URL: https://github.com/apache/commons-lang/pull/889 I added a method to help people convert any object to String or return an empty String. I added this methods because I was facing a problem with convert Wrap classes like Integer, Double to String. I needed something to return empty string or a default value. This is my contribute. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-lang] EACUAMBA closed pull request #888: Convert object to String.
EACUAMBA closed pull request #888: Convert object to String. URL: https://github.com/apache/commons-lang/pull/888 -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-lang] EACUAMBA opened a new pull request, #888: Convert object to String.
EACUAMBA opened a new pull request, #888: URL: https://github.com/apache/commons-lang/pull/888 I added two methods to convert Objects to String. I did it because I was facing a problem to convert Wraper objects to String and lead with NullPointerException. With this two method you can now pass any object and it will be converted to String or if this object is null will produce an empty String "". -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-bcel] timboudreau closed pull request #125: Fix IAE when calling `toString(ConstantPool)` on a `Module` or `ModuleRequires`
timboudreau closed pull request #125: Fix IAE when calling `toString(ConstantPool)` on a `Module` or `ModuleRequires` URL: https://github.com/apache/commons-bcel/pull/125 -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-bcel] timboudreau commented on pull request #125: Fix IAE when calling `toString(ConstantPool)` on a `Module` or `ModuleRequires`
timboudreau commented on PR #125: URL: https://github.com/apache/commons-bcel/pull/125#issuecomment-1114302035 The module info file in the test I provided _is_ a real module info file - just base-64 encoded. But whatever. Closing. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-lang] garydgregory merged pull request #886: Bump github/codeql-action from 1 to 2
garydgregory merged PR #886: URL: https://github.com/apache/commons-lang/pull/886 -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-bcel] garydgregory commented on pull request #125: Fix IAE when calling `toString(ConstantPool)` on a `Module` or `ModuleRequires`
garydgregory commented on PR #125: URL: https://github.com/apache/commons-bcel/pull/125#issuecomment-1114286993 Hi @timboudreau I appologize for not being clearer in my comment https://github.com/apache/commons-bcel/pull/125#issuecomment-1113740147 There is now a fix and a test in git master. If you think the change and/or the test in git master is incomplete, then feel free to update this PR or create a new one, whatever is simpler for you. I did not include a typecast because it was not required for this use case and none of the other cases in the switch of the affected method use a cast. Note that the test in git master include real module-info class files. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-bcel] timboudreau commented on pull request #125: Fix IAE when calling `toString(ConstantPool)` on a `Module` or `ModuleRequires`
timboudreau commented on PR #125: URL: https://github.com/apache/commons-bcel/pull/125#issuecomment-1114260729 I see a change that blindly casts to ConstantUtf8 has been made in the trunk, now creating a conflict with this PR. I have no idea if the blind cast is safe (it does not look like it), but regardless you may want to pick up the unit test from this PR so there is a way to detect regression. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-bcel] timboudreau commented on pull request #125: Fix IAE when calling `toString(ConstantPool)` on a `Module` or `ModuleRequires`
timboudreau commented on PR #125: URL: https://github.com/apache/commons-bcel/pull/125#issuecomment-1114259680 Updated the PR branch to no longer call `StringBuilder.isEmpty()` in tests. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (DBCP-585) Connection level JMX queries result in concurrent access to connection objects, causing errors
[ https://issues.apache.org/jira/browse/DBCP-585?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17530521#comment-17530521 ] Gary D. Gregory commented on DBCP-585: -- Hi [~psteitz] Do you think the PR's test is enough to validate the changes? > Connection level JMX queries result in concurrent access to connection > objects, causing errors > -- > > Key: DBCP-585 > URL: https://issues.apache.org/jira/browse/DBCP-585 > Project: Commons DBCP > Issue Type: Bug >Affects Versions: 2.9.0 >Reporter: Kurtcebe Eroglu >Priority: Major > Attachments: 0001-DBCP-585-idea-clarification-1.patch, > conn_instance_attrs.png, connections_attrs.png, ds_attrs.png, final.png > > Time Spent: 1h 10m > Remaining Estimate: 0h > > As we expose Connection objects over JMX, they may be accessed by multiple > threads concurrently; > a) an application thread that borrows the Connection and uses it business as > usual, > b) another thread simultaneously performing a JMX query, which in turn calls > getters on the same connection object via the MBean interface. > Also, calls to Connection object getters are mostly delegated to the > underlying vendor-specific connection provided by the JDBC driver. For > example, when we make the JMX query to get the "schema" attribute of the JMX > connection object, this is translated into a > "java.sql.Connection.getSchema()", and passed to the vendor-specific > Connection object by DBCP. In the case of Postgres, for example, this is > further translated to a query "select current_schema()" and sent to the > server. > Hence, querying connections over JMX result in concurrent access by multiple > threads to the underlying Connection provided by the vendors, to the point > that these two threads may be running queries simultaneously on the same > connection. > However, this is not supported by any of the major database vendors. Vendor > links on Connection objects not being threadsafe: > - [Postgres|https://jdbc.postgresql.org/documentation/head/thread.html] > {quote}The PostgreSQL™ JDBC driver is not thread safe. The PostgreSQL server > is not threaded. Each connection creates a new process on the server; as such > any concurrent requests to the process would have to be serialized. The > driver makes no guarantees that methods on connections are synchronized. It > will be up to the caller to synchronize calls to the driver. > {quote} > - > [Oracle|https://docs.oracle.com/en/database/oracle/oracle-database/19/jjdbc/JDBC-coding-tips.html#GUID-EE479007-D105-4F82-8D51-000CBBD4BC77] > > {quote}Oracle strongly discourages sharing a database connection among > multiple threads. Avoid allowing multiple threads to access a connection > simultaneously. > {quote} > - [Microsoft SQL > Server|https://www.javadoc.io/doc/com.microsoft.sqlserver/mssql-jdbc/latest/com.microsoft.sqlserver.jdbc/com/microsoft/sqlserver/jdbc/SQLServerConnection.html] > {quote}SQLServerConnection is not thread safe, however multiple statements > created from a single connection can be processing simultaneously in > concurrent threads. > {quote} > Another interesting point to note, also to do justice to previous committers > who have put this feature in place, is that this was not always the case. In > the following links, you may see the same links to the older versions of the > same pages. In the past, all vendors indicated that Connection is fully > thread-safe; [Postgres|https://www.postgresql.org/docs/7.1/jdbc-thread.html], > [Oracle|https://docs.oracle.com/cd/A97335_02/apps.102/a83724/tips1.htm], > [MSSQL > Server|https://www.javadoc.io/doc/com.microsoft.sqlserver/mssql-jdbc/6.1.0.jre7/com/microsoft/sqlserver/jdbc/SQLServerConnection.html]. > > Hence, it was once safe to expose Connection objects via JMX given the > thread-safety guarantees for the underlying vendor connection were in place. > But as Vendors dropped the thread-safety guarantee one by one, it is not safe > anymore, and may actually cause convoluted errors that pop up intermittently > due to thread races in the JDBC driver code (see an example in the comments > section below). Accordingly, exposing Connections via JMX shall be retired > along with dropped support from most vendors. > Note: the Datasource MBeans, which provide a vital set of metrics have no > such problems as they don't depend on the underlying JDBC provider. -- This message was sent by Atlassian Jira (v8.20.7#820007)
[GitHub] [commons-geometry] asfgit merged pull request #198: Addressing sonar issues
asfgit merged PR #198: URL: https://github.com/apache/commons-geometry/pull/198 -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (GEOMETRY-144) Review API in "hull" module
[ https://issues.apache.org/jira/browse/GEOMETRY-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17530519#comment-17530519 ] Matt Juntunen commented on GEOMETRY-144: I'm thinking it would be useful to apply the builder pattern for convex hull construction. The current API in {{commons-geometry-hull}} only accepts a collection of points, which means that if callers do not have such a collection readily available, they must allocate memory for the collection, extract points from whatever data structure they are part of, and _then_ pass it to the convex hull algorithm. If we use the builder pattern, we can avoid this allocation overhead. For example, {code:java} // previous API RegionBSPTree3D tree = ... List pts = new ArrayList<>(); tree.boundaryStream(b -> pts.addAll(pts.getVertices()); // pts could become quite large here ConvexHull3D hull = ... // create from pts // builder API RegionBSPTree3D tree = ... ConvexHull3D.Builder hullBuilder = ConvexHull3D.builder(precision); tree.boundaryStream(b -> hullBuilder.addAll(b.getVertices())); ConvexHull3D hull = hullBuilder.build(); {code} It may also be possible for us completely ignore particular points (and not even store them within the builder) if the point can be immediately determined to not lie on the hull. > Review API in "hull" module > --- > > Key: GEOMETRY-144 > URL: https://issues.apache.org/jira/browse/GEOMETRY-144 > Project: Commons Geometry > Issue Type: Task >Reporter: Gilles Sadowski >Assignee: Gilles Sadowski >Priority: Minor > Fix For: 1.1 > > > Review codes in the > [{{commons-geometry-hull}}|https://gitbox.apache.org/repos/asf?p=commons-geometry.git;a=tree;f=commons-geometry-hull;hb=HEAD] > module. > (x) Minimize the public API -- This message was sent by Atlassian Jira (v8.20.7#820007)
[GitHub] [commons-daemon] garydgregory commented on pull request #32: DAEMON-336 - If lCallbacks is empty don't call the fnCallbacks to avoid an ACCESS_VIOLATION
garydgregory commented on PR #32: URL: https://github.com/apache/commons-daemon/pull/32#issuecomment-1114211791 Any luck reproducing this issue or including changes for the upcoming RC? -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-daemon] garydgregory commented on a diff in pull request #39: Fix for https://issues.redhat.com/browse/JBCS-1261
garydgregory commented on code in PR #39: URL: https://github.com/apache/commons-daemon/pull/39#discussion_r862461720 ## src/native/windows/src/rprocess.c: ## @@ -316,13 +373,16 @@ static BOOL __apxProcessClose(APXHANDLE hProcess) lpProc = APXHANDLE_DATA(hProcess); CHECK_IF_ACTIVE(lpProc); +__apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, TRUE); /* Try to close the child's stdin first */ Review Comment: > Cool should I add a comment in the code to explain it traces the process tree and another where it kills the process tree? 👍 for comments. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Work logged] (IO-768) Add reserved Windows file names CONIN$ and CONOUT$ to FileSystem
[ https://issues.apache.org/jira/browse/IO-768?focusedWorklogId=764754&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-764754 ] ASF GitHub Bot logged work on IO-768: - Author: ASF GitHub Bot Created on: 01/May/22 09:19 Start Date: 01/May/22 09:19 Worklog Time Spent: 10m Work Description: michael-o commented on code in PR #355: URL: https://github.com/apache/commons-io/pull/355#discussion_r862447101 ## src/main/java/org/apache/commons/io/FileSystem.java: ## @@ -73,6 +73,8 @@ public enum FileSystem { * * @see https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file";>Naming Conventions * (microsoft.com) + * @see https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#consoles";> Review Comment: I think it is good. Since it is decent technical ref. Issue Time Tracking --- Worklog Id: (was: 764754) Time Spent: 20m (was: 10m) > Add reserved Windows file names CONIN$ and CONOUT$ to FileSystem > > > Key: IO-768 > URL: https://issues.apache.org/jira/browse/IO-768 > Project: Commons IO > Issue Type: Improvement >Affects Versions: 2.11.0 >Reporter: Marcono1234 >Priority: Minor > Time Spent: 20m > Remaining Estimate: 0h > > The file names {{CONIN$}} and {{CONOUT$}} are also reserved on Windows, see > https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#consoles. > Example code: > {code} > jshell> Files.readString(Path.of("CONIN$")) > | Exception java.io.IOException: Incorrect function > ... > {code} > It would be good to add them to {{org.apache.commons.io.FileSystem}}. -- This message was sent by Atlassian Jira (v8.20.7#820007)
[GitHub] [commons-io] michael-o commented on a diff in pull request #355: IO-768: Add reserved Windows file names CONIN$ and CONOUT$ to FileSystem
michael-o commented on code in PR #355: URL: https://github.com/apache/commons-io/pull/355#discussion_r862447101 ## src/main/java/org/apache/commons/io/FileSystem.java: ## @@ -73,6 +73,8 @@ public enum FileSystem { * * @see https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file";>Naming Conventions * (microsoft.com) + * @see https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#consoles";> Review Comment: I think it is good. Since it is decent technical ref. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-daemon] jfclere commented on a diff in pull request #39: Fix for https://issues.redhat.com/browse/JBCS-1261
jfclere commented on code in PR #39: URL: https://github.com/apache/commons-daemon/pull/39#discussion_r862443964 ## src/native/windows/src/rprocess.c: ## @@ -316,13 +373,16 @@ static BOOL __apxProcessClose(APXHANDLE hProcess) lpProc = APXHANDLE_DATA(hProcess); CHECK_IF_ACTIVE(lpProc); +__apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, TRUE); /* Try to close the child's stdin first */ Review Comment: Done Many thanks for the review. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-daemon] markt-asf commented on a diff in pull request #39: Fix for https://issues.redhat.com/browse/JBCS-1261
markt-asf commented on code in PR #39: URL: https://github.com/apache/commons-daemon/pull/39#discussion_r862440496 ## src/native/windows/src/rprocess.c: ## @@ -316,13 +373,16 @@ static BOOL __apxProcessClose(APXHANDLE hProcess) lpProc = APXHANDLE_DATA(hProcess); CHECK_IF_ACTIVE(lpProc); +__apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, TRUE); /* Try to close the child's stdin first */ Review Comment: It was obvious once you explained it but a comment wouldn't hurt. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-daemon] jfclere commented on a diff in pull request #39: Fix for https://issues.redhat.com/browse/JBCS-1261
jfclere commented on code in PR #39: URL: https://github.com/apache/commons-daemon/pull/39#discussion_r862440196 ## src/native/windows/src/rprocess.c: ## @@ -316,13 +373,16 @@ static BOOL __apxProcessClose(APXHANDLE hProcess) lpProc = APXHANDLE_DATA(hProcess); CHECK_IF_ACTIVE(lpProc); +__apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, TRUE); /* Try to close the child's stdin first */ Review Comment: Cool should I add a comment in the code to explain it traces the process tree and another where it kills the process tree? -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [commons-daemon] markt-asf commented on a diff in pull request #39: Fix for https://issues.redhat.com/browse/JBCS-1261
markt-asf commented on code in PR #39: URL: https://github.com/apache/commons-daemon/pull/39#discussion_r862439030 ## src/native/windows/src/rprocess.c: ## @@ -316,13 +373,16 @@ static BOOL __apxProcessClose(APXHANDLE hProcess) lpProc = APXHANDLE_DATA(hProcess); CHECK_IF_ACTIVE(lpProc); +__apxProcessTerminateChild(lpProc->stProcInfo.dwProcessId, TRUE); /* Try to close the child's stdin first */ Review Comment: Got it. It was `dryrun` being `TRUE` that I missed. This makes sense now. It generates a debug log of the current child processes before a normal shutdown. The forced shutdown only happens if the normal one goes wrong. LGTM. -- 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: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (JEXL-367) Deprecate -> and support =>
[ https://issues.apache.org/jira/browse/JEXL-367?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17530490#comment-17530490 ] Dmitri Blinov commented on JEXL-367: [~henrib] In short, in JS world fat arrow functions have their own ‘this’ scope, and don’t have bindings to ‘arguments’. There are also restrictions to use some operators etc. I think it’s pretty much documented in JS reference[JS reference|[https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions]|https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions].] The point is - arrow functions are not syntactic sugar for usual functions in JS. In JEXL world the differences above may not apply, since, for example, there is no ‘this’, yet. But as a thought - if we seriously decided to chase JS, IMO there are plenty of other missing features - operators, statements, constructor calls, varargs. For all it worth, I have had even added them in my fork the other day, so if anyone is interested please have a look. > Deprecate -> and support => > --- > > Key: JEXL-367 > URL: https://issues.apache.org/jira/browse/JEXL-367 > Project: Commons JEXL > Issue Type: Wish >Affects Versions: 3.2.1 >Reporter: Hussachai Puripunpinyo >Assignee: Henri Biestro >Priority: Major > > The JEXL code surprisingly looks a lot like Javascript. I think this change > is a good transition for folks to update the code, and it's pretty fine if > they can tolerate using the deprecate syntax and don't mind seeing a warning > log pop up every time. > I'd like to propose supporting => and deprecate ->. > The reasons are > - JavaScript becomes very popular and many people are familiar with it. > - JEXL is more like for a quick short script. In many scenarios, the target > audiences are not a programer. They often mistake a language as a JavaScript > (from my experience). > - JEXL syntax already looks a lot like JavaScript > -- var for variable declaration (Java added in Java 10, but JavaScript > supports this from the beginning) > -- The function keyword > -- Implicit type coercion > -- Ternary operator > The proposed change. > * Support => in addition to -> > * Deprecate -> and show a warning log when it's used. -- This message was sent by Atlassian Jira (v8.20.7#820007)