You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "elharo (via GitHub)" <gi...@apache.org> on 2023/10/08 13:01:00 UTC
[PR] replace a number of deprecated methods [commons-io]
elharo opened a new pull request, #495:
URL: https://github.com/apache/commons-io/pull/495
Most;y by adding explicit charsets, except in the cases where the omission of a charset was what was being tested
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace a number of deprecated methods [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #495:
URL: https://github.com/apache/commons-io/pull/495#issuecomment-1878624633
> We might end up setting up separate JDK 21 CIs, one with UTF-8 and one with something weird like Cp936, to flush out the issues.
That sounds good (and interesting) but this PR should be closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Replace a number of deprecated methods [commons-io]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #495:
URL: https://github.com/apache/commons-io/pull/495#issuecomment-1878722609
I need to take another look at this, but I'm not convinced the existing tests are correct or testing what they think they're testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] replace a number of deprecated methods [commons-io]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #495:
URL: https://github.com/apache/commons-io/pull/495#issuecomment-1752024602
## [Codecov](https://app.codecov.io/gh/apache/commons-io/pull/495?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#495](https://app.codecov.io/gh/apache/commons-io/pull/495?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (ee5c9db) into [master](https://app.codecov.io/gh/apache/commons-io/commit/43594ce6f5d5286677233442bf06b68a67874c3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (43594ce) will **decrease** coverage by `0.05%`.
> Report is 2 commits behind head on master.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #495 +/- ##
============================================
- Coverage 84.97% 84.92% -0.05%
+ Complexity 3372 3370 -2
============================================
Files 227 227
Lines 8080 8080
Branches 953 953
============================================
- Hits 6866 6862 -4
- Misses 960 964 +4
Partials 254 254
```
[see 1 file with indirect coverage changes](https://app.codecov.io/gh/apache/commons-io/pull/495/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] replace a number of deprecated methods [commons-io]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #495:
URL: https://github.com/apache/commons-io/pull/495#issuecomment-1752031897
I think in all cases here each method still tests what it tested before. Only setup and verification code has been rewritten to use non-deprecated methods and to be more platform independent. There were a couple places I left unchanged precisely because they test deprecated method. If I missed one of those, point to it and I'll fix it.
Code coverage numbers are not accurate to three significant figures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] replace a number of deprecated methods [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #495:
URL: https://github.com/apache/commons-io/pull/495#issuecomment-1752044512
We need to keep the code as is to show it work on Java 18 and up where the default encoding is UTF-8. We should consider undeprecating these methods on Java 18 and up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] replace a number of deprecated methods [commons-io]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #495:
URL: https://github.com/apache/commons-io/pull/495#issuecomment-1752028403
-1: We have to test ALL the code we provide. Also, notice this PR _decreases_ the code coverage percentage:
```
@@ Coverage Diff @@
## master #495 +/- ##
============================================
- Coverage 84.97% 84.92% -0.05%
+ Complexity 3372 3370 -2
============================================
Files 227 227
Lines 8080 8080
Branches 953 953
============================================
- Hits 6866 6862 -4
- Misses 960 964 +4
Partials 254 254
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] replace a number of deprecated methods [commons-io]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #495:
URL: https://github.com/apache/commons-io/pull/495#issuecomment-1752152568
That would be nice. Unfortunately, "An implementation may override the default charset with the system property file.encoding on the command line. If the value is COMPAT, the default charset is derived from the native.encoding system property, which typically depends upon the locale and charset of the underlying operating system." so we're going to need to specify charsets for the next 25 years. :-(
We might end up setting up separate JDK 21 CIs, one with UTF-8 and one with something weird like Cp936, to flush out the issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org