Locale root

[arsen-es]

Issue #, if available: N/A

Description of changes: Just creating utility methods not to make everyone use Locale.ROOT all the time. Plus fixed some warnings. Replaced all the usages of String.format in test classes, someone (maybe me) will need to do for non-test as well.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[arsen-es]

Sorry for the big change, I just had it done a while ago and now want to clean-up my local branches, so just decided to push.

[galkk]

I think that for most of the cases (especially tests) we shouldn't use format at all.

For example:

        String result = explain(format("{\"query\":\"" +
                "SELECT * " +	                "SELECT * " +
                "FROM %s " +	                "FROM %s " +
                "WHERE firstname LIKE 'A%%' AND age > 20 " +	                "WHERE firstname LIKE 'A%%' AND age > 20 " +
                "GROUP BY gender " +	                "GROUP BY gender " +
                "ORDER BY _score\"}", TestsConstants.TEST_INDEX_ACCOUNT));	

We don't need format here at all, we can just add index name into the query.

Proposal:

Split this PR into 2: StringUtils + the places where format is strictly necessary (non-test codebase, exception strings), and the rest (unit tests). The former PR will go easily, because it's needed, for the latter we can continue our conversation if format is actually needed there at all

[arsen-es]

I think that for most of the cases (especially tests) we shouldn't use format at all.

For example:

        String result = explain(format("{\"query\":\"" +
                "SELECT * " +	                "SELECT * " +
                "FROM %s " +	                "FROM %s " +
                "WHERE firstname LIKE 'A%%' AND age > 20 " +	                "WHERE firstname LIKE 'A%%' AND age > 20 " +
                "GROUP BY gender " +	                "GROUP BY gender " +
                "ORDER BY _score\"}", TestsConstants.TEST_INDEX_ACCOUNT));	

We don't need format here at all, we can just add index name into the query.

Proposal:

Split this PR into 2: StringUtils + the places where format is strictly necessary (non-test codebase, exception strings), and the rest (unit tests). The former PR will go easily, because it's needed, for the latter we can continue our conversation if format is actually needed there at all

1. I fixed the line length for everything going beyond 120 characters. I fixed some unused import warnings, and some others as well. So if I revert my change, I will lose all those fixes.
2. The issue you describe comes because most of our tests are a result of migration, and not written from scratch by us, and as long as they tested the functionality they were good enough. Bringing their code to a very good shape is indeed a useful thing, but I think in terms of priority that will be the last thing to address.
3. Removing the format calls is independent of this change, if we decide to do it, it has to be done anyway, so it's not that my work is wasted, at least not for the time until we remove it. Another point is when we add a new test we usually copy the skeleton code from an existing one, and having the String.format call in one place might propagate to the new tests.

So I still suggest I address all the other comments and push the full change. Whenever and whoever decides to address your concern about using format where it's not necessary or even discouraged, they can do that change. Addressing that change as 2nd part of this PR adds a lot of extra work, which I don't have time for right now, since there are higher priority items to work on.

@dai-chen, @abbashus , @penghuo - what do you think? If you are in agreement with Andy's proposal, I will just keep the StringUtils addition along with unit tests, and revert all the other changes.

[dai-chen]

I think that for most of the cases (especially tests) we shouldn't use format at all.
For example:

        String result = explain(format("{\"query\":\"" +
                "SELECT * " +	                "SELECT * " +
                "FROM %s " +	                "FROM %s " +
                "WHERE firstname LIKE 'A%%' AND age > 20 " +	                "WHERE firstname LIKE 'A%%' AND age > 20 " +
                "GROUP BY gender " +	                "GROUP BY gender " +
                "ORDER BY _score\"}", TestsConstants.TEST_INDEX_ACCOUNT));	

We don't need format here at all, we can just add index name into the query.
Proposal:
Split this PR into 2: StringUtils + the places where format is strictly necessary (non-test codebase, exception strings), and the rest (unit tests). The former PR will go easily, because it's needed, for the latter we can continue our conversation if format is actually needed there at all

1. I fixed the line length for everything going beyond 120 characters. I fixed some unused import warnings, and some others as well. So if I revert my change, I will lose all those fixes.
2. The issue you describe comes because most of our tests are a result of migration, and not written from scratch by us, and as long as they tested the functionality they were good enough. Bringing their code to a very good shape is indeed a useful thing, but I think in terms of priority that will be the last thing to address.
3. Removing the format calls is independent of this change, if we decide to do it, it has to be done anyway, so it's not that my work is wasted, at least not for the time until we remove it. Another point is when we add a new test we usually copy the skeleton code from an existing one, and having the String.format call in one place might propagate to the new tests.

So I still suggest I address all the other comments and push the full change. Whenever and whoever decides to address your concern about using format where it's not necessary or even discouraged, they can do that change. Addressing that change as 2nd part of this PR adds a lot of extra work, which I don't have time for right now, since there are higher priority items to work on.

@dai-chen, @abbashus , @penghuo - what do you think? If you are in agreement with Andy's proposal, I will just keep the StringUtils addition along with unit tests, and revert all the other changes.

I'm fine with either way (check in what you have now or revert) because we haven't figured out how to do it much better. Concat many pieces of string or hardcode entire query, both look not perfect to me. I think we can continue the discussion on another Issue/PR.

[arsen-es]

Checked in the StringUtils class in a separate PR (#107), so this one now is pending the decision whether to update all usages of String.format with the new format method in the tests. Please see former conversation for more details.

[arsen-es]

Closing this PR, since the essential part was already checked in as part of #107, and this code is outdated.