tengqm commented on code in PR #5992:
URL: https://github.com/apache/gravitino/pull/5992#discussion_r1897634614
##########
catalogs/catalog-common/src/main/java/org/apache/gravitino/storage/GCSProperties.java:
##########
@@ -22,7 +22,7 @@
public class GCSProperties {
// The path of service account JSON file of Google Cloud Storage.
- public static final String GCS_SERVICE_ACCOUNT_JSON_PATH =
"gcs-service-account-file";
+ public static final String GRAVITINO_GCS_SERVICE_ACCOUNT_FILE =
"gcs-service-account-file";
Review Comment:
@FANNG1 Yes. That was also my first perception of this naming style after
having seen it prevail in many packages. But I'm still confused by such `public
static final string`s.
You know, we are writing these code in Java, which is notorious/famous by
its class hierarchies. In other words, we are not supposed to write two `Foo`
classes in the same package. Each `Foo` has its own namespace. A `Foo` can be a
Gravitino `Foo` and it can be a Iceberg `Foo`. We differentiate these two `Foo`
classes by using their fully-qualified names. We are not supposed to add
another level of complexity to the class hierarchy.
For most of the cases, a `org.gravitino.Foo.CONFIG_FILE` can be easily
differentiated from a `org.iceberg.Foo.CONFIG_FILE`. Prepending `GRAVATINO_` or
`ICEBERG_` to those variable/constant names is making them longer for no good.
If these code is written in Go, for example, I'd strongly advise the other way
around. A prefix can help us quickly locate the file where it is defined.
Without a prefix, I have to grep the whole directory because GoLang compiler
treats the whole directory as a single compilation unit. A variable can be
defined in any source code. That design sucks.
I have seen simply too many cases where our contributors are repeating
(copy-pasting) this pattern, and that is the reason I'm putting this forward
for the team to reconsider, if appropriate.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
