You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Donal Evans <do...@pivotal.io> on 2020/05/18 01:49:09 UTC

PR to replace every use of hard-coded / character with a constant

Following on from my first pass refactor a couple of weeks ago, there is
now a PR up for review which replaces every instance of a hard-coded "/"
character in region names/paths (outside of xml files used by tests and the
oql.g file used to parse queries) with a reference to a constant:
https://github.com/apache/geode/pull/5127

The changes touch 880 files, so the PR is broken up by module/package, in
case anyone really wants to review specific areas, but the main structural
changes can be found in the first commit
<https://github.com/apache/geode/pull/5127/commits/52de83dce664ea2af5f92f16febbb9c3e8318272>
in
the PR.

To verify that all changes are correct and that no instances of "/" in
region names/paths have been missed, I replaced the character in
GeodePublicGlossary with a "~", updated oql.g to allow queries to be
correctly parsed using the new character, and ran all tests on a draft PR
branch. All failing tests were either directly comparing the region
separator to "/", creating caches from XML files containing regions/indexes
using "/," or involved older client/server versions which would not be
compatible with the changed separator, so I am confident that no non-region
slashes have been changed, and none missed.

Hopefully, if this PR is approved and merged, we can establish a totally
consistent convention of always using the constant in region names/paths
going forward.