refactor entity parser #398
Conversation
| indexType = reflect.TypeOf((*Index)(nil)).Elem() | ||
| tagsKey = []string{"primaryKey", "key", "name", "columns", "etl", "ttl"} |
There was a problem hiding this comment.
I see the same strings are repeated in the below code, which can be error-prone. How about defining them as variables like:
primaryKeyTag = "primaryKey";
and use "primaryKeyTag" in the remaining places so that compiler will tell you whether or not you are spelling correctly.
| return "", nil, nil, fmt.Errorf("index field %s with an invalid dosa index tag: %s", indexName, tag) | ||
| var key *PrimaryKey | ||
| if val, ok := tagsMap["key"]; ok { | ||
| key, err = parsePrimaryKey(val) |
There was a problem hiding this comment.
Given that we use the same function for paring primary keys and index keys, how about calling it "parseKey"
| fullNameTag = matches[0] | ||
| name = matches[1] | ||
| // parseSinglePrimaryKey func parses the single parimary key of DOSA object | ||
| func parseSinglePrimaryKey(s string) (*PrimaryKey, error) { |
| // parseSinglePrimaryKey func parses the single parimary key of DOSA object | ||
| func parseSinglePrimaryKey(s string) (*PrimaryKey, error) { | ||
| s = strings.TrimSpace(s) | ||
| spaceIdx := strings.Index(s, " ") |
There was a problem hiding this comment.
Will this impact existing users?
| // parseCompoundPartitionKeys func parses the compund partition key of DOSA object | ||
| func parseCompoundPartitionKeys(s string) ([]string, error) { | ||
| s = strings.TrimSpace(strings.Trim(s, "()")) | ||
| fields := strings.Split(s, ",") |
There was a problem hiding this comment.
do we need to check for the existence of spaces like the function above?
| for _, ckField := range ckFields { | ||
| fields := strings.Fields(ckField) | ||
| if len(fields) == 0 { | ||
| continue |
|
Gang Zhang seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
No description provided.