[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16592505#comment-16592505 ] ASF GitHub Bot commented on IMAGING-154: Github user asfgit closed the pull request at: https://github.com/apache/commons-imaging/pull/35 > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16590305#comment-16590305 ] ASF GitHub Bot commented on IMAGING-154: Github user andreasrosdal commented on the issue: https://github.com/apache/commons-imaging/pull/35 I hope that you will publish the release to the maven central repos, since I hope to use it as a dependency. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16590248#comment-16590248 ] ASF GitHub Bot commented on IMAGING-154: Github user kinow commented on the issue: https://github.com/apache/commons-imaging/pull/35 Thanks @garydgregory ! No objection for a 1.0-alpha1, then wait a couple of months or a bit more perhaps for user feedback. I assume this 1.0-alpha1 would make all the way to maven central? > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16590218#comment-16590218 ] ASF GitHub Bot commented on IMAGING-154: Github user garydgregory commented on the issue: https://github.com/apache/commons-imaging/pull/35 +1 to merge. I'd prefer to see a 1.0-alpha1 or 1.0-beta1 released next instead of 1.0 but I'm not RM'ing this one... ;-) > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16590081#comment-16590081 ] ASF GitHub Bot commented on IMAGING-154: Github user kinow commented on the issue: https://github.com/apache/commons-imaging/pull/35 Thanks @andreasrosdal ! I still have some more development bandwidth this month, so hopefully it will be merged this weekend, and I will find a way to squeeze in a few more hours to finish the website updates. Then we will be ready to call a 1.0 vote again. @garydgregory ping. I left a few replies to your comments. I think that'd be OK to merge as is, unless you prefer to address the exception handling & code duplication now. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16585563#comment-16585563 ] ASF GitHub Bot commented on IMAGING-154: Github user andreasrosdal commented on the issue: https://github.com/apache/commons-imaging/pull/35 I hope we can get a 1.0 release of Commons Imaging very soon. Thanks for working towards this! > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16583574#comment-16583574 ] ASF GitHub Bot commented on IMAGING-154: Github user kinow commented on the issue: https://github.com/apache/commons-imaging/pull/35 Agreed on not using INFO level @sebbASF & @garydgregory . Moved what was in FINE to FINEST level. And moved what was in INFO to FINE. My plan for the web site information is to have the following information, just with more complete information: >we are using jul > >exceptions will be logged to SEVERE >a more verbose output is available at FINE >nformation for debug and troubleshooting is available at FINEST @garydgregory addressed your comments about code duplication with comments above. If there are no other objections here or in the mailing list, I am planning to merge this work next weekend (that should give plenty of time for others to review, and time for me to try to work on the **last** blocking issue for 1.0: documentation! Thanks! > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582461#comment-16582461 ] ASF GitHub Bot commented on IMAGING-154: Github user sebbASF commented on the issue: https://github.com/apache/commons-imaging/pull/35 Agreed > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582453#comment-16582453 ] ASF GitHub Bot commented on IMAGING-154: Github user garydgregory commented on the issue: https://github.com/apache/commons-imaging/pull/35 In general I do not think a library should log at INFO, only DEBUG or TRACE. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582335#comment-16582335
]
ASF GitHub Bot commented on IMAGING-154:
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210549560
--- Diff:
src/main/java/org/apache/commons/imaging/formats/bmp/BmpImageParser.java ---
@@ -327,7 +331,7 @@ private BmpImageContents readImageContents(final
InputStream is,
switch (bhi.compression) {
case BI_RGB:
if (verbose) {
--- End diff --
And on
>(wonder if we should use FINEST for debug, and FINE for the verbose
messages...)
If you have the chance to run the tests, you will notice the huge amount of
verbose messages being printed now. I believe some users could be inundated
with log lines (as the INFO is enabled by default? and verbose was disabled by
default)
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582327#comment-16582327
]
ASF GitHub Bot commented on IMAGING-154:
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210546814
--- Diff:
src/main/java/org/apache/commons/imaging/formats/bmp/BmpImageParser.java ---
@@ -327,7 +331,7 @@ private BmpImageContents readImageContents(final
InputStream is,
switch (bhi.compression) {
case BI_RGB:
if (verbose) {
--- End diff --
Done in
https://github.com/apache/commons-imaging/pull/35/commits/96b0a18e3c0ac435da85a3cf361975e2e77aa7f0
Also removed the `PARAM_KEY_VERBOSE` constant, used to include a param in a
map to enable verbose. I initially put a `@deprecated` annotation, then
realized this will be 1.0 of imaging, not sanselan, so removing it should be OK.
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582238#comment-16582238
]
ASF GitHub Bot commented on IMAGING-154:
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210524716
--- Diff: src/main/java/org/apache/commons/imaging/icc/IccTag.java ---
@@ -73,11 +76,12 @@ private IccTagDataType getIccTagDataType(final int
quad) {
}
public void dump(final String prefix) throws ImageReadException,
IOException {
-final PrintWriter pw = new PrintWriter(new
OutputStreamWriter(System.out, Charset.defaultCharset()));
-
-dump(pw, prefix);
-
-pw.flush();
+try (StringWriter sw = new StringWriter(); PrintWriter pw = new
PrintWriter(sw)) {
--- End diff --
This was intentional.
The previous code contains the `throws IOException` (while the other
methods don't) ->
https://github.com/apache/commons-imaging/blob/d2ec76bd10f30c39ae5180ede1254908e76045f0/src/main/java/org/apache/commons/imaging/icc/IccTag.java#L75
Should we make them all the same, catching and logging, instead of throwing
the IOException? As this is a `dump` method, not used by another method such as
`toString()`, I think the impact on end users should be minimal.
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582230#comment-16582230
]
ASF GitHub Bot commented on IMAGING-154:
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210523866
--- Diff:
src/main/java/org/apache/commons/imaging/formats/psd/PsdHeaderInfo.java ---
@@ -45,9 +50,14 @@ public PsdHeaderInfo(final int version, final byte[]
reserved, final int channel
}
public void dump() {
-final PrintWriter pw = new PrintWriter(new
OutputStreamWriter(System.out, Charset.defaultCharset()));
-dump(pw);
-pw.flush();
+try (StringWriter sw = new StringWriter(); PrintWriter pw = new
PrintWriter(sw)) {
--- End diff --
@garydgregory I'm not sure how to refactor it. If you look at the `dump`
methods, you will see they are scattered all over the place, but there's no
`@Override` tags, as they are not defined in any parent class.
I am ignoring this for now, as we could have users coming from Sanselan
using these methods. But for 2.x I would like to tackle this problem and either
remove the `dump` methods, or have a design with a base class defining the
signature of these methods.
That would give us a place to implement a method that could remove this
duplication.
The alternative that I see here, is to add a `Util` class to the project,
and add a method to remove the duplication. But I am not very fond of `Util`
classes for a case like this one (where a proper design would remove the need
for it).
I hope this makes sense. Do you think we could ignore this duplication for
now, and fix in the upcoming (hopefully soon) next releases?
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582219#comment-16582219
]
ASF GitHub Bot commented on IMAGING-154:
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210521304
--- Diff:
src/main/java/org/apache/commons/imaging/formats/bmp/BmpImageParser.java ---
@@ -327,7 +331,7 @@ private BmpImageContents readImageContents(final
InputStream is,
switch (bhi.compression) {
case BI_RGB:
if (verbose) {
--- End diff --
You are both correct @garydgregory , @sebbASF .
I tried to remove the flags `DEBUG` or `isDebug()` or `getDebug()` by
`LOGGER.isLoggable(Level.FINE)`, but forgot to do the same for `verbose` using
the `INFO` level (wonder if we should use FINEST for debug, and FINE for the
verbose messages...)
Will just log to info. I believe we can assume that
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16582210#comment-16582210
]
ASF GitHub Bot commented on IMAGING-154:
Github user kinow commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210519489
--- Diff:
src/main/java/org/apache/commons/imaging/formats/png/chunks/PngChunkText.java
---
@@ -42,9 +47,9 @@ public PngChunkText(final int length, final int
chunkType, final int crc, final
final int textLength = bytes.length - (index + 1);
text = new String(bytes, index + 1, textLength,
StandardCharsets.ISO_8859_1);
-if (getDebug()) {
-System.out.println("Keyword: " + keyword);
-System.out.println("Text: " + text);
+if (LOGGER.isLoggable(Level.FINE)) {
--- End diff --
I went straight for `LOGGER.isDebugEnabled()` and the IDE remembered me
what it actually looked like :=/
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581313#comment-16581313
]
ASF GitHub Bot commented on IMAGING-154:
Github user sebbASF commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210327438
--- Diff:
src/main/java/org/apache/commons/imaging/formats/bmp/BmpImageParser.java ---
@@ -327,7 +331,7 @@ private BmpImageContents readImageContents(final
InputStream is,
switch (bhi.compression) {
case BI_RGB:
if (verbose) {
--- End diff --
Seems to me it would be better to use the log-level alone to control what
is being logged.
So the verbose flag would become a log-level instead.
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581152#comment-16581152 ] ASF GitHub Bot commented on IMAGING-154: Github user garydgregory commented on a diff in the pull request: https://github.com/apache/commons-imaging/pull/35#discussion_r210287763 --- Diff: src/test/java/org/apache/commons/imaging/formats/jpeg/exif/WriteExifMetadataExampleTest.java --- @@ -27,7 +27,7 @@ import org.apache.commons.imaging.formats.jpeg.JpegImageParser; import org.apache.commons.imaging.formats.tiff.TiffField; import org.apache.commons.imaging.formats.tiff.TiffImageMetadata; -import org.apache.commons.imaging.util.Debug; +import org.apache.commons.imaging.internal.Debug; --- End diff -- I like the package name :-) > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581150#comment-16581150
]
ASF GitHub Bot commented on IMAGING-154:
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210287591
--- Diff: src/main/java/org/apache/commons/imaging/icc/IccTag.java ---
@@ -73,11 +76,12 @@ private IccTagDataType getIccTagDataType(final int
quad) {
}
public void dump(final String prefix) throws ImageReadException,
IOException {
-final PrintWriter pw = new PrintWriter(new
OutputStreamWriter(System.out, Charset.defaultCharset()));
-
-dump(pw, prefix);
-
-pw.flush();
+try (StringWriter sw = new StringWriter(); PrintWriter pw = new
PrintWriter(sw)) {
--- End diff --
Same comment as above even though this code is slightly different - no
catch clause.
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581147#comment-16581147
]
ASF GitHub Bot commented on IMAGING-154:
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210287176
--- Diff:
src/main/java/org/apache/commons/imaging/formats/psd/PsdImageContents.java ---
@@ -40,9 +45,14 @@ public PsdImageContents(final PsdHeaderInfo header,
}
public void dump() {
-final PrintWriter pw = new PrintWriter(new
OutputStreamWriter(System.out, Charset.defaultCharset()));
-dump(pw);
-pw.flush();
+try (StringWriter sw = new StringWriter(); PrintWriter pw = new
PrintWriter(sw)) {
--- End diff --
Same comment as above.
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581149#comment-16581149
]
ASF GitHub Bot commented on IMAGING-154:
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210287298
--- Diff:
src/main/java/org/apache/commons/imaging/formats/tiff/TiffField.java ---
@@ -355,9 +360,14 @@ private String getValueDescription(final Object o) {
}
public void dump() {
-final PrintWriter pw = new PrintWriter(new
OutputStreamWriter(System.out, Charset.defaultCharset()));
-dump(pw);
-pw.flush();
+try (StringWriter sw = new StringWriter(); PrintWriter pw = new
PrintWriter(sw)) {
--- End diff --
Same comment as above.
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581146#comment-16581146
]
ASF GitHub Bot commented on IMAGING-154:
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210287038
--- Diff:
src/main/java/org/apache/commons/imaging/formats/psd/PsdHeaderInfo.java ---
@@ -45,9 +50,14 @@ public PsdHeaderInfo(final int version, final byte[]
reserved, final int channel
}
public void dump() {
-final PrintWriter pw = new PrintWriter(new
OutputStreamWriter(System.out, Charset.defaultCharset()));
-dump(pw);
-pw.flush();
+try (StringWriter sw = new StringWriter(); PrintWriter pw = new
PrintWriter(sw)) {
--- End diff --
This seems like duplicate code from a similar block I saw above. Should it
be refactored?
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581143#comment-16581143
]
ASF GitHub Bot commented on IMAGING-154:
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210286764
--- Diff:
src/main/java/org/apache/commons/imaging/formats/png/chunks/PngChunkText.java
---
@@ -42,9 +47,9 @@ public PngChunkText(final int length, final int
chunkType, final int crc, final
final int textLength = bytes.length - (index + 1);
text = new String(bytes, index + 1, textLength,
StandardCharsets.ISO_8859_1);
-if (getDebug()) {
-System.out.println("Keyword: " + keyword);
-System.out.println("Text: " + text);
+if (LOGGER.isLoggable(Level.FINE)) {
--- End diff --
Uhg, the JUL level names are AWFUL :=(
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[
https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16581139#comment-16581139
]
ASF GitHub Bot commented on IMAGING-154:
Github user garydgregory commented on a diff in the pull request:
https://github.com/apache/commons-imaging/pull/35#discussion_r210286324
--- Diff:
src/main/java/org/apache/commons/imaging/formats/bmp/BmpImageParser.java ---
@@ -327,7 +331,7 @@ private BmpImageContents readImageContents(final
InputStream is,
switch (bhi.compression) {
case BI_RGB:
if (verbose) {
--- End diff --
Seeing "if verbose" and then "log at the info level" is slightly confusing
here. If we want to keep the 'verbose' setting then maybe the code should
rename this to 'logInfo', or, should we provide a setting that contains the log
level to use when 'verbose' is enabled?
> Remove Debug class
> --
>
> Key: IMAGING-154
> URL: https://issues.apache.org/jira/browse/IMAGING-154
> Project: Commons Imaging
> Issue Type: Task
>Affects Versions: Patch Needed
>Reporter: Benedikt Ritter
>Assignee: Bruno P. Kinoshita
>Priority: Major
> Fix For: 1.0
>
>
> Low level libraries should not do logging, but communicate through the use of
> exceptions and meaningful return values. Remove the Debug class and all it's
> uses.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16577458#comment-16577458 ] Bruno P. Kinoshita commented on IMAGING-154: So went ahead to re-design the Debug class, in a way users could still enable/disable debugging, and also use a PrintStream so that other thing rather than System.out could be used. Then, realized removing System.out was the natural next step. But alas, the library uses System.out for debugging, but sometimes it uses it for writing to System.out in a "verbose mode". What is more complicated, is that sometimes classes methods like `toString()` are calling debug methods that receive a PrintStream already. So I spent some more time quickly comparing what other libraries I've seen being used / or used for image processing: [https://kinoshita.eti.br/2018/08/12/use-of-logging-in-java-image-processing-libraries/.|https://kinoshita.eti.br/2018/08/12/use-of-logging-in-java-image-processing-libraries/] Turns out only very low level libraries, such as the JNI bridge for OpenCV, im4java, and Java's ImageIO can do with just throwing Exception's. All other libraries have one way or another of logging. Be it with JUL, SLF4J, custom loggers, or with the ol' System.out/err. My preferred compromise for this ticket was to keep Debug, making System.out possible but optional, and mark the class internal only. Now my preferred solution is to keep the Debug internal, but add a logger to it. And then also add logging to replace where System.out is used for the "verbose" mode. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16577364#comment-16577364 ] Bruno P. Kinoshita commented on IMAGING-154: For the record, previous e-mail thread: [https://markmail.org/thread/ak3hcka7piykxixz#query:+page:1+mid:ppgxbhjx3opqlixj+state:results] Going to spend some time re-reading the complete ticket, comments, and e-mail thread, as well as my old pull request, and try to think how to address this again either today (Sunday my time) or until the next weekend. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16352302#comment-16352302 ] Bruno P. Kinoshita commented on IMAGING-154: Hi Gilles, I think this was one of the blockers for 1.0. But you are right, there was no discussion in the mailing list. My guess is that we won't reach consensus easily. Initially my preference is always to remove logging/debugging from commons libraries, unless really necessary or useful. And I will admit that some time ago when I tried to troubleshoot TIFF issues, the dump/debug features helped immensely. Said that, I could achieve the same with a bit more of time, and some help of the Eclipse debugger and a piece of paper and pen. I'm OK with a solution like Log4J, especially if it provides a good replacement for the dump methods. So let me start a discussion in the mailing list to see what others think, and postpone merging the PR :) Thanks! B > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16352290#comment-16352290 ] Gilles commented on IMAGING-154: Hi Bruno. Was there a discussion about this that concluded to a "simple" removal? Just quickly browsing through the changes, it seems (?) that obtaining debug info requires either access to the internals or, for example, implementing bit-masking gymnastics (which is tedious and error-prone). Input to this library's code is not only through passing known parameters (args to method) but also reading "external" data (image file); so I guess that in some cases of malformed (or unknown) input, logging is an immense help. A flexible alternative to the current "dump" is to use a logging interface such as provided by [Log4J2|http://logging.apache.org/log4j/2.x/manual/api.html]. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16352265#comment-16352265 ] Bruno P. Kinoshita commented on IMAGING-154: Pull request submitted to give some time for others to review it. Will merge it in the next days if there are no objections. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16352263#comment-16352263 ] ASF GitHub Bot commented on IMAGING-154: Github user kinow commented on the issue: https://github.com/apache/commons-imaging/pull/35 `changes.xml` updated. Last commit contains the word-trick to close the pull request. Once merged, everything should be correct, just need to update JIRA. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16352262#comment-16352262 ] ASF GitHub Bot commented on IMAGING-154: GitHub user kinow opened a pull request: https://github.com/apache/commons-imaging/pull/35 IMAGING-154 Removed Debug class Removed the `Debug` and `ImageDump` classes. Also removed methods with the `verbose` parameter, as well as any `System.out.println`'s. Tried to keep the change to a minimum, but given the extent of its use, I believe the pull request diff will be a bit long. You can merge this pull request into a Git repository by running: $ git pull https://github.com/kinow/commons-imaging IMAGING-154 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/commons-imaging/pull/35.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 #35 commit 0e218c63086ce0e0ee9270d6099cc9466a0cda4c Author: Bruno P. Kinoshita Date: 2018-02-05T11:10:25Z IMAGING-154 Removed Debug class > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter >Assignee: Bruno P. Kinoshita >Priority: Major > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14263292#comment-14263292 ] Benedikt Ritter commented on IMAGING-154: - Part of this issue is to replace all the dump methods with something more useful that doesn't just print to std out. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (IMAGING-154) Remove Debug class
[ https://issues.apache.org/jira/browse/IMAGING-154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14263254#comment-14263254 ] Benedikt Ritter commented on IMAGING-154: - This is a show stopper for 1.0 for me. > Remove Debug class > -- > > Key: IMAGING-154 > URL: https://issues.apache.org/jira/browse/IMAGING-154 > Project: Commons Imaging > Issue Type: Task >Affects Versions: Patch Needed >Reporter: Benedikt Ritter > Fix For: 1.0 > > > Low level libraries should not do logging, but communicate through the use of > exceptions and meaningful return values. Remove the Debug class and all it's > uses. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
