[GitHub] commons-imaging issue #29: Improve various unit tests style
Github user jbduncan commented on the issue: https://github.com/apache/commons-imaging/pull/29 Looks much better, thanks @TheRealHaui! As I discussed in #28, I'm sure there's area for improvement on the whitespace/readability front, but that would require running `mvn checkstyle:check` and fixing the warnings it reports in a separate PR. On a related note, Travis seems to complaining that GreyScaleRountripTest (unrelated to this PR AFAICT) killed the JVM process during testing. Perhaps Travis just needs to be restarted and the test will run properly? But I don't immediately know how one would restart Travis... --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text issue #56: add-some-Unit Tests
Github user jbduncan commented on the issue: https://github.com/apache/commons-text/pull/56 @TheRealHaui Yes, precisely. :) (With hindsight, I should have used the word "violations" instead of "cases"; it would have been clearer that I was referring to the errors that Checkstyle reports, rather than something else.) --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text issue #56: add-some-Unit Tests
Github user jbduncan commented on the issue: https://github.com/apache/commons-text/pull/56 Thanks @TheRealHaui! I think handling the rest of the cases reported by Checkstyle should be done in a separate PR (or series of PRs if needed) by whoever feels up to 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text issue #56: add-some-Unit Tests
Github user jbduncan commented on the issue: https://github.com/apache/commons-text/pull/56 Oh sorry @TheRealHaui, I just reread my main comment in my review from a few minutes ago, and I realise that I may have sounded confrontational. I had not meant to come across as such, so I wish to apologise if it sounded that way. ð --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127854299
--- Diff:
src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java ---
@@ -55,11 +55,11 @@ public void testExtendedFormats() {
final String pattern = "Lower: {0,lower} Upper: {1,upper}";
final ExtendedMessageFormat emf = new
ExtendedMessageFormat(pattern, registry);
assertEquals("TOPATTERN", pattern, emf.toPattern());
-assertEquals("Lower: foo Upper: BAR", emf.format(new Object[]
{"foo", "bar"}));
-assertEquals("Lower: foo Upper: BAR", emf.format(new Object[]
{"Foo", "Bar"}));
-assertEquals("Lower: foo Upper: BAR", emf.format(new Object[]
{"FOO", "BAR"}));
-assertEquals("Lower: foo Upper: BAR", emf.format(new Object[]
{"FOO", "bar"}));
-assertEquals("Lower: foo Upper: BAR", emf.format(new Object[]
{"foo", "BAR"}));
+assertEquals("Lower: foo Upper: BAR", emf.format(new
Object[]{"foo", "bar"}));
+assertEquals("Lower: foo Upper: BAR", emf.format(new
Object[]{"Foo", "Bar"}));
+assertEquals("Lower: foo Upper: BAR", emf.format(new
Object[]{"FOO", "BAR"}));
+assertEquals("Lower: foo Upper: BAR", emf.format(new
Object[]{"FOO", "bar"}));
+assertEquals("Lower: foo Upper: BAR", emf.format(new
Object[]{"foo", "BAR"}));
--- End diff --
Unless whitespace changes like this one are required by Checkstyle, please
consider reverting them to prevent polluting the Git history.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127854120
--- Diff: src/test/java/org/apache/commons/text/AlphabetConverterTest.java
---
@@ -201,4 +207,97 @@ private void test(final Character[] originalChars,
final Character[] encodingCha
Assert.assertEquals("Encoded '" + s + "' into '" + encoded +
"', but decoded into '" + decoded + "'", s, decoded);
}
}
+
+@Test
+public void testCreateConverterFromCharsWithNullAndNull() {
+
+Character[] characterArray = new Character[2];
+characterArray[0] = Character.valueOf('$');
+characterArray[1] = characterArray[0];
+
+try {
+AlphabetConverter.createConverterFromChars(characterArray,
null, null);
+fail("Expecting exception: IllegalArgumentException");
+} catch (IllegalArgumentException e) {
+assertEquals(AlphabetConverter.class.getName(),
e.getStackTrace()[0].getClassName());
+}
+
--- End diff --
Blank lines at the end of methods, like this one, seem inconsistent with
the rest of the code base.
Please consider removing them from _all_ methods which you've created and
touched.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127855372
--- Diff:
src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java ---
@@ -426,10 +517,13 @@ public void testSetFormatsByArgumentIndex() {
@Override
public StringBuffer format(final Object obj, final StringBuffer
toAppendTo, final FieldPosition pos) {
-return toAppendTo.append(((String)obj).toLowerCase());
+return toAppendTo.append(((String) obj).toLowerCase());
--- End diff --
Whitespace changes like this are _good_ IMO, as they improve code clarity.
Well done! ð
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127856024
--- Diff:
src/test/java/org/apache/commons/text/similarity/LevenshteinResultsTest.java ---
@@ -0,0 +1,92 @@
+/*
+ * 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.text.similarity;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Unit tests for class {@link LevenshteinResults}.
+ *
+ * @date 2017-07-17
+ * @see LevenshteinResults
+ **/
--- End diff --
I struggle to see how this Javadoc is useful. Please consider removing 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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request: https://github.com/apache/commons-text/pull/56#discussion_r127854963 --- Diff: src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java --- @@ -353,10 +355,11 @@ private void checkBuiltInFormat(final String pattern, final Map<String, ?> fmtRe /** * Create an ExtendedMessageFormat for the specified pattern and locale and check the * formated output matches the expected result for the parameters. - * @param pattern string + * + * @param patternstring --- End diff -- Ditto. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request: https://github.com/apache/commons-text/pull/56#discussion_r127854991 --- Diff: src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java --- @@ -353,10 +355,11 @@ private void checkBuiltInFormat(final String pattern, final Map<String, ?> fmtRe /** * Create an ExtendedMessageFormat for the specified pattern and locale and check the * formated output matches the expected result for the parameters. - * @param pattern string + * + * @param patternstring * @param registryUnused map (currently unused) - * @param args Object[] - * @param locale Locale + * @param args Object[] + * @param locale Locale --- End diff -- Ditto. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request: https://github.com/apache/commons-text/pull/56#discussion_r127854930 --- Diff: src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java --- @@ -338,10 +339,11 @@ private void checkBuiltInFormat(final String pattern, final Object[] args, final /** * Test a built in format for the specified Locales, plus null Locale. - * @param pattern MessageFormat pattern + * + * @param pattern MessageFormat pattern --- End diff -- I think this line was fine before, please consider reverting 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request: https://github.com/apache/commons-text/pull/56#discussion_r127854888 --- Diff: src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java --- @@ -338,10 +339,11 @@ private void checkBuiltInFormat(final String pattern, final Object[] args, final /** * Test a built in format for the specified Locales, plus null Locale. - * @param pattern MessageFormat pattern + * + * @param pattern MessageFormat pattern * @param fmtRegistry FormatFactory registry to use - * @param args MessageFormat arguments - * @param locales to test + * @param argsMessageFormat arguments + * @param locales to test --- End diff -- I think the lines were fine before, please consider reverting them. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request: https://github.com/apache/commons-text/pull/56#discussion_r127855769 --- Diff: src/test/java/org/apache/commons/text/StrSubstitutorTest.java --- @@ -17,21 +17,16 @@ package org.apache.commons.text; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import org.apache.commons.lang3.mutable.MutableObject; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; import java.util.HashMap; import java.util.Map; import java.util.Properties; -import org.apache.commons.lang3.mutable.MutableObject; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; +import static org.junit.Assert.*; --- End diff -- I think the use of non-wildcard `Assert.assert*` imports was fine before. Please consider expanding these imports back into proper ones. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127855540
--- Diff:
src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java ---
@@ -476,8 +578,8 @@ public Format getFormat(final String name, final String
arguments, final Locale
public Format getFormat(final String name, final String arguments,
final Locale locale) {
return !"short".equals(arguments) ? null
: locale == null ? DateFormat
-.getDateInstance(DateFormat.DEFAULT) :
DateFormat
-.getDateInstance(DateFormat.DEFAULT, locale);
+.getDateInstance(DateFormat.DEFAULT) : DateFormat
+.getDateInstance(DateFormat.DEFAULT, locale);
--- End diff --
I think these lines were reasonably indented before. Unless Checkstyle
complains, please consider reverting these lines.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127855137
--- Diff:
src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java ---
@@ -373,14 +376,15 @@ private void checkBuiltInFormat(final String pattern,
final Map<String, ?> regis
} else {
emf = new ExtendedMessageFormat(pattern, locale);
}
-assertEquals("format "+ buffer.toString(), mf.format(args),
emf.format(args));
+assertEquals("format " + buffer.toString(), mf.format(args),
emf.format(args));
assertEquals("toPattern " + buffer.toString(), mf.toPattern(),
emf.toPattern());
}
/**
* Replace MessageFormat(String, Locale) constructor (not available
until JDK 1.4).
+ *
* @param pattern string
- * @param locale Locale
+ * @param locale Locale
--- End diff --
Ditto.
Please consider reverting whitespace changes for _all_ `@param` lines where
you only inserted whitespaces between the parameter name and its description.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127854015
--- Diff: src/test/java/org/apache/commons/text/AlphabetConverterTest.java
---
@@ -201,4 +207,97 @@ private void test(final Character[] originalChars,
final Character[] encodingCha
Assert.assertEquals("Encoded '" + s + "' into '" + encoded +
"', but decoded into '" + decoded + "'", s, decoded);
}
}
+
+@Test
+public void testCreateConverterFromCharsWithNullAndNull() {
+
--- End diff --
Blank lines at the start of methods, like this one, seem inconsistent with
the rest of the code base.
Please consider removing them from _all_ methods which you've created and
touched.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127854827
--- Diff:
src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java ---
@@ -295,41 +295,42 @@ public void testEqualsHashcode() {
ExtendedMessageFormat other = null;
// Same object
-assertTrue("same, equals()", emf.equals(emf));
+assertTrue("same, equals()", emf.equals(emf));
assertTrue("same, hashcode()", emf.hashCode() == emf.hashCode());
assertFalse("null, equals", emf.equals(null));
// Equal Object
other = new ExtendedMessageFormat(pattern, Locale.US, fmtRegistry);
-assertTrue("equal, equals()", emf.equals(other));
+assertTrue("equal, equals()", emf.equals(other));
assertTrue("equal, hashcode()", emf.hashCode() ==
other.hashCode());
// Different Class
other = new OtherExtendedMessageFormat(pattern, Locale.US,
fmtRegistry);
-assertFalse("class, equals()", emf.equals(other));
+assertFalse("class, equals()", emf.equals(other));
assertTrue("class, hashcode()", emf.hashCode() ==
other.hashCode()); // same hashcode
// Different pattern
other = new ExtendedMessageFormat("X" + pattern, Locale.US,
fmtRegistry);
-assertFalse("pattern, equals()", emf.equals(other));
+assertFalse("pattern, equals()", emf.equals(other));
assertFalse("pattern, hashcode()", emf.hashCode() ==
other.hashCode());
// Different registry
other = new ExtendedMessageFormat(pattern, Locale.US,
otherRegitry);
-assertFalse("registry, equals()", emf.equals(other));
+assertFalse("registry, equals()", emf.equals(other));
assertFalse("registry, hashcode()", emf.hashCode() ==
other.hashCode());
// Different Locale
other = new ExtendedMessageFormat(pattern, Locale.FRANCE,
fmtRegistry);
-assertFalse("locale, equals()", emf.equals(other));
+assertFalse("locale, equals()", emf.equals(other));
assertTrue("locale, hashcode()", emf.hashCode() ==
other.hashCode()); // same hashcode
}
/**
* Test a built in format for the specified Locales, plus
null Locale.
+ *
* @param pattern MessageFormat pattern
- * @param args MessageFormat arguments
+ * @param argsMessageFormat arguments
--- End diff --
I think this line was fine before. Please consider reverting 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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text pull request #56: add-some-Unit Tests
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-text/pull/56#discussion_r127855604
--- Diff:
src/test/java/org/apache/commons/text/ExtendedMessageFormatTest.java ---
@@ -488,10 +590,10 @@ public Format getFormat(final String name, final
String arguments, final Locale
private static final long serialVersionUID = 1L;
public OtherExtendedMessageFormat(final String pattern, final
Locale locale,
-final Map<String, ? extends FormatFactory> registry) {
+ final Map<String, ? extends
FormatFactory> registry) {
--- End diff --
I think these lines were reasonably indented before. Unless Checkstyle
complains, please consider reverting these lines.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127780127
--- Diff: src/test/java/org/apache/commons/imaging/ImageDumpTest.java ---
@@ -0,0 +1,44 @@
+package org.apache.commons.imaging;
+
+import org.junit.Test;
+
+import java.awt.color.ColorSpace;
+import java.awt.image.BufferedImage;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for class {@link ImageDump}.
+ *
+ * @date 2017-07-13
+ * @see ImageDump
+ *
+ **/
--- End diff --
I question the presence of Javadoc in this class and all the other classes
affected by this PR. AFAICT not all the tests (if any) even have Javadocs, so
why should javadocs be added for this class and the others?
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127782716
--- Diff:
src/test/java/org/apache/commons/imaging/common/bytesource/ByteSourceTest.java
---
@@ -17,24 +17,22 @@
package org.apache.commons.imaging.common.bytesource;
-import static org.junit.Assert.assertTrue;
+import org.apache.commons.imaging.ImagingTest;
+import org.junit.Test;
-import java.io.BufferedOutputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
+import java.io.*;
-import org.apache.commons.imaging.ImagingTest;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
public abstract class ByteSourceTest extends ImagingTest {
protected File createTempFile(final byte src[]) throws IOException {
final File file = createTempFile("raw_", ".bin");
// write test bytes to file.
try (FileOutputStream fos = new FileOutputStream(file);
-OutputStream os = new BufferedOutputStream(fos)) {
+ OutputStream os = new BufferedOutputStream(fos)) {
--- End diff --
No need to un-tab this line AFAICT.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127781644
--- Diff: src/test/java/org/apache/commons/imaging/ImageDumpTest.java ---
@@ -0,0 +1,44 @@
+package org.apache.commons.imaging;
+
+import org.junit.Test;
+
+import java.awt.color.ColorSpace;
+import java.awt.image.BufferedImage;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for class {@link ImageDump}.
+ *
+ * @date 2017-07-13
+ * @see ImageDump
+ *
+ **/
+public class ImageDumpTest{
+
+
+@Test
+public void testDumpColorSpace() {
+
+ImageDump imageDump = new ImageDump();
+ColorSpace colorSpace = ColorSpace.getInstance(1004);
+imageDump.dumpColorSpace("Ku&]N>!4'C#Jzn+", colorSpace);
+
+assertEquals(3, colorSpace.getNumComponents());
+
+}
+
+
+@Test
+public void testDump() {
+
+ImageDump imageDump = new ImageDump();
+BufferedImage bufferedImage = new BufferedImage(10, 10, 10);
+imageDump.dump(bufferedImage);
+
+assertEquals(10, bufferedImage.getHeight());
+
+}
+
+
--- End diff --
Having two blank lines here seems to be inconsistent with the other test
classes in commons-imaging. I suggest shortening it to one or zero blank lines.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127780455
--- Diff: src/test/java/org/apache/commons/imaging/ImageDumpTest.java ---
@@ -0,0 +1,44 @@
+package org.apache.commons.imaging;
+
+import org.junit.Test;
+
+import java.awt.color.ColorSpace;
+import java.awt.image.BufferedImage;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for class {@link ImageDump}.
+ *
+ * @date 2017-07-13
+ * @see ImageDump
+ *
+ **/
+public class ImageDumpTest{
--- End diff --
The `{` seems to be joined directly with the class name to its left. This
is inconsistent with all the other test classes AFAICT.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127782811
--- Diff:
src/test/java/org/apache/commons/imaging/common/bytesource/ByteSourceTest.java
---
@@ -66,6 +64,22 @@ protected File createTempFile(final byte src[]) throws
IOException {
}
final byte longArray[] = (baos.toByteArray());
-return new byte[][] { emptyArray, single, simple, zeroes,
longArray, };
+return new byte[][]{emptyArray, single, simple, zeroes,
longArray,};
--- End diff --
No need to remove spaces here AFAICT.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127782170
--- Diff:
src/test/java/org/apache/commons/imaging/common/RgbBufferedImageFactoryTest.java
---
@@ -0,0 +1,36 @@
+package org.apache.commons.imaging.common;
+
+import org.junit.Test;
+
+import java.awt.image.DirectColorModel;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests for class {@link RgbBufferedImageFactory}.
+ *
+ * @date 2017-07-13
+ * @see RgbBufferedImageFactory
+ *
+ **/
+public class RgbBufferedImageFactoryTest{
+
+
+@Test
+public void testGetColorBufferedImageThrowsIllegalArgumentException() {
+
+RgbBufferedImageFactory rgbBufferedImageFactory = new
RgbBufferedImageFactory();
+
+try {
+rgbBufferedImageFactory.getColorBufferedImage(0, 0, true);
+fail("Expecting exception: IllegalArgumentException");
+} catch(IllegalArgumentException e) {
+assertEquals("Width (0) and height (0) cannot be <=
0",e.getMessage());
+assertEquals(DirectColorModel.class.getName(),
e.getStackTrace()[0].getClassName());
+}
+
--- End diff --
Ditto regarding blank lines. Please shorten them for all other classes as
well.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127783301
--- Diff:
src/test/java/org/apache/commons/imaging/formats/jpeg/iptc/IptcTypeLookupTest.java
---
@@ -0,0 +1,28 @@
+package org.apache.commons.imaging.formats.jpeg.iptc;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for class {@link IptcTypeLookup}.
+ *
+ * @date 2017-07-13
+ * @see IptcTypeLookup
+ *
+ **/
+public class IptcTypeLookupTest{
+
+
+@Test
+public void testGetIptcTypeWithPositive() {
+
+IptcType iptcType = IptcTypeLookup.getIptcType(1117);
+
+assertEquals( 1117, iptcType.getType() );
+assertEquals( "Unknown", iptcType.getName() );
--- End diff --
Ditto regarding spaces at start and end of parameter lists.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127783226
--- Diff:
src/test/java/org/apache/commons/imaging/formats/bmp/BmpWriterRgbTest.java ---
@@ -0,0 +1,35 @@
+package org.apache.commons.imaging.formats.bmp;
+
+import org.junit.Test;
+
+import java.awt.image.BufferedImage;
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for class {@link BmpWriterRgb}.
+ *
+ * @date 2017-07-13
+ * @see BmpWriterRgb
+ *
+ **/
+public class BmpWriterRgbTest{
+
+
+@Test
+public void testGetImageData() {
+
+BmpWriterRgb bmpWriterRgb = new BmpWriterRgb();
+BufferedImage bufferedImage = new BufferedImage(2, 2, 5);
+byte[] byteArray = bmpWriterRgb.getImageData(bufferedImage);
+
+assertEquals( 24, bmpWriterRgb.getBitsPerPixel() );
+assertEquals( 0, bmpWriterRgb.getPaletteSize() );
+assertEquals( 16, byteArray.length );
+assertEquals( "[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]",
Arrays.toString(byteArray) );
--- End diff --
Spaces at start and end of parameter lists seem unnecessary 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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request: https://github.com/apache/commons-imaging/pull/28#discussion_r127782622 --- Diff: src/test/java/org/apache/commons/imaging/common/bytesource/ByteSourceTest.java --- @@ -17,24 +17,22 @@ package org.apache.commons.imaging.common.bytesource; -import static org.junit.Assert.assertTrue; +import org.apache.commons.imaging.ImagingTest; +import org.junit.Test; -import java.io.BufferedOutputStream; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.OutputStream; +import java.io.*; -import org.apache.commons.imaging.ImagingTest; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; --- End diff -- I'd keep the ordering and style of the imports as before; no need to rearrange them just to import `assertEquals` and `fail`. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127781095
--- Diff: src/test/java/org/apache/commons/imaging/ImageDumpTest.java ---
@@ -0,0 +1,44 @@
+package org.apache.commons.imaging;
+
+import org.junit.Test;
+
+import java.awt.color.ColorSpace;
+import java.awt.image.BufferedImage;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for class {@link ImageDump}.
+ *
+ * @date 2017-07-13
+ * @see ImageDump
+ *
+ **/
+public class ImageDumpTest{
+
+
+@Test
+public void testDumpColorSpace() {
+
+ImageDump imageDump = new ImageDump();
+ColorSpace colorSpace = ColorSpace.getInstance(1004);
+imageDump.dumpColorSpace("Ku&]N>!4'C#Jzn+", colorSpace);
+
+assertEquals(3, colorSpace.getNumComponents());
+
+}
+
+
+@Test
+public void testDump() {
+
+ImageDump imageDump = new ImageDump();
+BufferedImage bufferedImage = new BufferedImage(10, 10, 10);
+imageDump.dump(bufferedImage);
+
+assertEquals(10, bufferedImage.getHeight());
+
+}
--- End diff --
Ditto regarding blank lines.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127781035
--- Diff: src/test/java/org/apache/commons/imaging/ImageDumpTest.java ---
@@ -0,0 +1,44 @@
+package org.apache.commons.imaging;
+
+import org.junit.Test;
+
+import java.awt.color.ColorSpace;
+import java.awt.image.BufferedImage;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for class {@link ImageDump}.
+ *
+ * @date 2017-07-13
+ * @see ImageDump
+ *
+ **/
+public class ImageDumpTest{
+
+
+@Test
+public void testDumpColorSpace() {
+
+ImageDump imageDump = new ImageDump();
+ColorSpace colorSpace = ColorSpace.getInstance(1004);
+imageDump.dumpColorSpace("Ku&]N>!4'C#Jzn+", colorSpace);
+
+assertEquals(3, colorSpace.getNumComponents());
+
+}
--- End diff --
Most test methods I've seen in commons-imaging prefer to have no blank
lines at the start and end of the test method body. I suggest removing the
blank lines at the start and end of `testDumpColorSpace`'s body here for
consistency, so like this:
```java
@Test
public void testDumpColorSpace() {
ImageDump imageDump = new ImageDump();
ColorSpace colorSpace = ColorSpace.getInstance(1004);
imageDump.dumpColorSpace("Ku&]N>!4'C#Jzn+", colorSpace);
assertEquals(3, colorSpace.getNumComponents());
}
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127781676
--- Diff:
src/test/java/org/apache/commons/imaging/common/RgbBufferedImageFactoryTest.java
---
@@ -0,0 +1,36 @@
+package org.apache.commons.imaging.common;
+
+import org.junit.Test;
+
+import java.awt.image.DirectColorModel;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests for class {@link RgbBufferedImageFactory}.
+ *
+ * @date 2017-07-13
+ * @see RgbBufferedImageFactory
+ *
+ **/
--- End diff --
Ditto regarding javadoc.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127782199
--- Diff:
src/test/java/org/apache/commons/imaging/common/RgbBufferedImageFactoryTest.java
---
@@ -0,0 +1,36 @@
+package org.apache.commons.imaging.common;
+
+import org.junit.Test;
+
+import java.awt.image.DirectColorModel;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests for class {@link RgbBufferedImageFactory}.
+ *
+ * @date 2017-07-13
+ * @see RgbBufferedImageFactory
+ *
+ **/
+public class RgbBufferedImageFactoryTest{
+
+
+@Test
+public void testGetColorBufferedImageThrowsIllegalArgumentException() {
+
+RgbBufferedImageFactory rgbBufferedImageFactory = new
RgbBufferedImageFactory();
+
+try {
+rgbBufferedImageFactory.getColorBufferedImage(0, 0, true);
+fail("Expecting exception: IllegalArgumentException");
+} catch(IllegalArgumentException e) {
+assertEquals("Width (0) and height (0) cannot be <=
0",e.getMessage());
+assertEquals(DirectColorModel.class.getName(),
e.getStackTrace()[0].getClassName());
+}
+
+}
+
+
--- End diff --
Ditto regarding blank lines. Please shorten them for all other classes as
well.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127781398
--- Diff: src/test/java/org/apache/commons/imaging/ImageDumpTest.java ---
@@ -0,0 +1,44 @@
+package org.apache.commons.imaging;
+
+import org.junit.Test;
+
+import java.awt.color.ColorSpace;
+import java.awt.image.BufferedImage;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Unit tests for class {@link ImageDump}.
+ *
+ * @date 2017-07-13
+ * @see ImageDump
+ *
+ **/
+public class ImageDumpTest{
+
+
--- End diff --
Having two blank lines here seems to be inconsistent with the other test
classes in commons-imaging. I suggest shortening it to one blank line.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127782141
--- Diff:
src/test/java/org/apache/commons/imaging/common/RgbBufferedImageFactoryTest.java
---
@@ -0,0 +1,36 @@
+package org.apache.commons.imaging.common;
+
+import org.junit.Test;
+
+import java.awt.image.DirectColorModel;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests for class {@link RgbBufferedImageFactory}.
+ *
+ * @date 2017-07-13
+ * @see RgbBufferedImageFactory
+ *
+ **/
+public class RgbBufferedImageFactoryTest{
+
+
+@Test
+public void testGetColorBufferedImageThrowsIllegalArgumentException() {
+
--- End diff --
Ditto regarding blank lines. Please shorten them for all other classes as
well.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #28: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/28#discussion_r127781871
--- Diff:
src/test/java/org/apache/commons/imaging/common/RgbBufferedImageFactoryTest.java
---
@@ -0,0 +1,36 @@
+package org.apache.commons.imaging.common;
+
+import org.junit.Test;
+
+import java.awt.image.DirectColorModel;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+/**
+ * Unit tests for class {@link RgbBufferedImageFactory}.
+ *
+ * @date 2017-07-13
+ * @see RgbBufferedImageFactory
+ *
+ **/
+public class RgbBufferedImageFactoryTest{
+
+
--- End diff --
Ditto regarding blank lines. Please shorten them for all other classes as
well.
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-imaging pull request #27: Increase code coverage one
Github user jbduncan commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/27#discussion_r127778422
--- Diff:
src/test/java/org/apache/commons/imaging/common/bytesource/ByteSourceTest.java
---
@@ -66,6 +64,22 @@ protected File createTempFile(final byte src[]) throws
IOException {
}
final byte longArray[] = (baos.toByteArray());
-return new byte[][] { emptyArray, single, simple, zeroes,
longArray, };
+return new byte[][]{emptyArray, single, simple, zeroes,
longArray,};
+}
+
+@Test
--- End diff --
@onealj @TheRealHaui If I were maintaining this code, I'd be happy with
either pattern.
`@Test(expected=NullPointerException.class)` would be a little shorter and
equivalent to the `try..catch` construct used here in this case.
However, the reason people tend to prefer `try..catch` is because in a lot
of cases it is not granular enough. Take this (admittedly contrived) example:
```java
@Test(expected=NullPointerException.class)
public void doSomething() {
String s = null;
assertTrue(s.isEmpty());
try {
somethingWhichThrowsNullPointerException():
} catch (NullPointerException e) {
assertNull(s);
}
}
```
In this case, rather than `somethingWhichThrowsNullPointerException(s1)`
throwing `NullPointerException` (which we expect), it will be `s.isEmpty()`
that throws it instead, which isn't what we intended, since it means the test
will always "pass".
---
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.
---
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text issue #49: TEXT-89: UTF-32 support for WordUtils.initials using...
Github user jbduncan commented on the issue: https://github.com/apache/commons-text/pull/49 It's not clear to me why the ordering of the imports has changed and why all the `assert*()` method imports have been replaced with a wildcard import. Are those intentional? --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...
Github user jbduncan commented on the issue: https://github.com/apache/commons-text/pull/55 Revisiting this in 2.x sounds good to me! The only other thought I have is that I think this should go up as a new issue on Apache JIRA if it hasn't already. Other than that, everything LGTM. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[GitHub] commons-text issue #55: TEXT-97: RandomStringGenerator able to pass multiple...
Github user jbduncan commented on the issue: https://github.com/apache/commons-text/pull/55 @ameyjadiye I don't know if there's a convention in Apache Commons for preferring `new X.Builder()` over static factory methods like `X.builder()`, but if there isn't, then I would encourage changing the API so that instead of `new RandomStringGenerator.Builder()`, one can write the shorter and arguably more readable `RandomStringGenerator.builder()`. --- 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. --- - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
