Re: No more hardcoded region separators!
Sadly not, unless it was incredibly convoluted and complex. There are plenty of String literal "/" still in the codebase, in URIs/URLs, filepaths and log output (for example "Updated 5/6 values") so it's not really possible to determine if the presence of a "/" is "correct" without looking at the actual code and how it's used. From: Murtuza Boxwala Sent: Thursday, May 28, 2020 10:37 AM To: dev@geode.apache.org Subject: Re: No more hardcoded region separators! Is there any way to enforce that with some kind of LGTM or spotless rule? On 5/28/20, 12:46 PM, "Donal Evans" wrote: I'm happy to say that as of about 5 minutes ago, there are no uses of hardcoded "/" in region paths/names in the geode codebase, as all of them have been replaced by the Region.SEPARATOR constant (with the exception of a few occurrences in the geode-management module, which while not having an explicit dependency on geode-core has implicit dependencies on some things like the region separator, index types etc). I'd like to request that going forward, we maintain this convention of only using Region.SEPARATOR and avoid introduction of any new hardcoded "/" characters in code pertaining to region paths or names, both in our own commits and in commits we review from other developers.
Re: No more hardcoded region separators!
Is there any way to enforce that with some kind of LGTM or spotless rule? On 5/28/20, 12:46 PM, "Donal Evans" wrote: I'm happy to say that as of about 5 minutes ago, there are no uses of hardcoded "/" in region paths/names in the geode codebase, as all of them have been replaced by the Region.SEPARATOR constant (with the exception of a few occurrences in the geode-management module, which while not having an explicit dependency on geode-core has implicit dependencies on some things like the region separator, index types etc). I'd like to request that going forward, we maintain this convention of only using Region.SEPARATOR and avoid introduction of any new hardcoded "/" characters in code pertaining to region paths or names, both in our own commits and in commits we review from other developers.
Re: No more hardcoded region separators!
Thanks for the suggestion, Dave. I'll be sure to add something soon. From: Dave Barnes Sent: Thursday, May 28, 2020 10:32 AM To: dev@geode.apache.org Subject: Re: No more hardcoded region separators! Excellent, Donal! If you have not already done so, please consider documenting the practice you're advocating in a place where all community contributors have a chance of seeing it. Maybe https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FHow%2Bto%2BContributedata=02%7C01%7Cdoevans%40vmware.com%7Cff0d3580ae404c540c9a08d8032d27ad%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637262839764198680sdata=CattsXf3p8X%2BiFalU2uu9be6nxdzLkJ4dXCfL7tE0ec%3Dreserved=0? On Thu, May 28, 2020 at 9:46 AM Donal Evans wrote: > I'm happy to say that as of about 5 minutes ago, there are no uses of > hardcoded "/" in region paths/names in the geode codebase, as all of them > have been replaced by the Region.SEPARATOR constant (with the exception of > a few occurrences in the geode-management module, which while not having an > explicit dependency on geode-core has implicit dependencies on some things > like the region separator, index types etc). I'd like to request that going > forward, we maintain this convention of only using Region.SEPARATOR and > avoid introduction of any new hardcoded "/" characters in code pertaining > to region paths or names, both in our own commits and in commits we review > from other developers. >
Re: No more hardcoded region separators!
Excellent, Donal! If you have not already done so, please consider documenting the practice you're advocating in a place where all community contributors have a chance of seeing it. Maybe https://cwiki.apache.org/confluence/display/GEODE/How+to+Contribute? On Thu, May 28, 2020 at 9:46 AM Donal Evans wrote: > I'm happy to say that as of about 5 minutes ago, there are no uses of > hardcoded "/" in region paths/names in the geode codebase, as all of them > have been replaced by the Region.SEPARATOR constant (with the exception of > a few occurrences in the geode-management module, which while not having an > explicit dependency on geode-core has implicit dependencies on some things > like the region separator, index types etc). I'd like to request that going > forward, we maintain this convention of only using Region.SEPARATOR and > avoid introduction of any new hardcoded "/" characters in code pertaining > to region paths or names, both in our own commits and in commits we review > from other developers. >
No more hardcoded region separators!
I'm happy to say that as of about 5 minutes ago, there are no uses of hardcoded "/" in region paths/names in the geode codebase, as all of them have been replaced by the Region.SEPARATOR constant (with the exception of a few occurrences in the geode-management module, which while not having an explicit dependency on geode-core has implicit dependencies on some things like the region separator, index types etc). I'd like to request that going forward, we maintain this convention of only using Region.SEPARATOR and avoid introduction of any new hardcoded "/" characters in code pertaining to region paths or names, both in our own commits and in commits we review from other developers.
