[GitHub] nifi issue #2800: NIFI-5317 - support non-ASCII X-Mailer header

2018-06-17 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/nifi/pull/2800 This looks good, just two comments. 1. If we are going to add support for non-ascii, the test should test non-ascii, can we add that? 2. We now have a pattern of use in two places us

[GitHub] nifi issue #2800: NIFI-5317 - support non-ASCII X-Mailer header

2018-06-17 Thread dtrodrigues
Github user dtrodrigues commented on the issue: https://github.com/apache/nifi/pull/2800 Header setting is now refactored into a function and there are tests for non-ascii headers ---

[GitHub] nifi issue #2800: NIFI-5317 - support non-ASCII X-Mailer header

2018-06-17 Thread dtrodrigues
Github user dtrodrigues commented on the issue: https://github.com/apache/nifi/pull/2800 I updated the test X-Mailer header to be more explicit that it's non-ASCII ---

[GitHub] nifi issue #2800: NIFI-5317 - support non-ASCII X-Mailer header

2018-06-17 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/nifi/pull/2800 Yikes, sorry. ---

[GitHub] nifi issue #2800: NIFI-5317 - support non-ASCII X-Mailer header

2018-06-17 Thread ottobackwards
Github user ottobackwards commented on the issue: https://github.com/apache/nifi/pull/2800 +1 LGTM, mvn install and contrib-check pass, use of MineType utility per best practices. ---

[GitHub] nifi issue #2800: NIFI-5317 - support non-ASCII X-Mailer header

2018-06-18 Thread pvillard31
Github user pvillard31 commented on the issue: https://github.com/apache/nifi/pull/2800 +1, merging to master, thanks @dtrodrigues @ottobackwards ---