[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-521614090 @nickwallen anything you need to see before we wrap up this PR? This

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-521360960 @mmiklavc I've added an upgrading note. Let me know if I'm missing anything there. As a reminder to whoever merges this in (ad

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-521040026 @mmiklavc If they're using a system default that isn't UTF-8 (say ISO_8859_1), all the parsers that currently work would silently sta

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-521029586 @mmiklavc We should probably add a note to the Upgrading.md. There is a backwards compatibility concern when upgrading: When

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-521011869 Merged. Adds some docs and the test fix. @mmiklavc Assuming the build succeeds, are we good for @nickwallen + @ottobackwards t

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-520965262 @mmiklavc Unit test looks like it fails because a mocked parser NPEs on the `getReadCharset`. Could just do a `when(broParser.getRea

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-520927745 The PR from @mmiklavc (https://github.com/justinleet/metron/pull/22) got merged to address the couple `readCharset` issues.

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-519913565 @mmiklavc I merged in your PR against this branch. I'll add any commentary here as I review, but obviously feel encouraged to run thr

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-515071778 Apparently I built skipping tests, I'll take a look when I have a chance

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-514831259 @mmiklavc Merged in master, fixed the incoming issues that would have caused problems, built successfully, and ran up full dev and ma

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-514669137 @mmiklavc I'll try to get to it by the end of the week. I had a personal emergency come up last week, and unfortunately this fell pre

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-510943635 @mmiklavc Sure, I'll deconflict and get you set up. Definitely would appreciate the help; I've been too busy to come back to this and

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-493480413 Ahh good call, it's been long enough that I'd forgotten about that discussion. There should definitely at least be a README addition

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-493453296 @mmiklavc @nickwallen @ottobackwards I merged master post-release and ran up in full dev again. Is everyone still good with the chang

[GitHub] [metron] justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset

justinleet commented on issue #1341: METRON-614: Eliminate use of the default Charset URL: https://github.com/apache/metron/pull/1341#issuecomment-486854994 Fixed the conflicts and ran back up in full dev. Having said that, I'd like to hold off on merging this until the release is over, s