Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

future proof method that is marked for incompatible change #6

Merged
merged 10 commits into from
Jul 27, 2020
Merged

Conversation

elharo
Copy link
Contributor

@elharo elharo commented Jul 21, 2020

and generally be explicit about which dependencies this depends on

Copy link
Contributor

@dennisl dennisl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When platform encoding is being used, this change removed it from being printed to the log. This is useful ingformation to users that don't know what encoding is being used.
This can be accessed by simply calling "System.getProperty( "file.encoding" )" instead of "ReaderFactory.FILE_ENCODING".

@elharo
Copy link
Contributor Author

elharo commented Jul 21, 2020

Is it useful though? The relevant detail is that the platform default is being used. The point is that whatever value that is, and which we would print here, is not reliable or consistent.

@dennisl
Copy link
Contributor

dennisl commented Jul 21, 2020

In my experience it is very useful. When moving a project from being built locally by developers to a CI solution, developers are often surprised when they don't get the same results there.

@elharo
Copy link
Contributor Author

elharo commented Jul 21, 2020

Yes, and when that happens they should sit down and figure out which encoding they should use (usually UTF-8). They should not simply copy and paste whichever encoding they happened to be using by accident like they're writing a characterization test to mimic the idiosyncratic behavior of their development box.

@dennisl
Copy link
Contributor

dennisl commented Jul 21, 2020 via email

Copy link
Contributor Author

@elharo elharo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, added the encoding back in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants