-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
d2c024c
to
b7768b2
Compare
handlers/namespaces.go
Outdated
return 0, nil, err | ||
} | ||
version := value.Version() | ||
services := &schema.Namespaces{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename services
to namespaces
since this is in OSS world and namespace
is more generic than service
.
Also, more idiomatic to write
var namespaces schema.Namespaces
if err := value.Unmarshal(&namespaces); err != nil {
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bump
handlers/namespaces.go
Outdated
return namespacesVersion, namespaces, ns, nil | ||
} | ||
|
||
// Namespace returns the service with a given name, or an error if there are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
returns the namespace with a given name
handlers/namespaces.go
Outdated
return nil, errMultipleMatches | ||
} | ||
} | ||
return namespace, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps better to return errNotFound
if the namespace is nil here?
handlers/namespaces.go
Outdated
if ns == nil { | ||
return 0, nil, nil, fmt.Errorf("namespace %s doesn't exist", namespaceName) | ||
} | ||
if ns.Snapshots[len(ns.Snapshots)-1].Tombstoned { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be safer to check if len(ns.Snapshots)
is nonzero and return an error if so.
handlers/ruleset.go
Outdated
rollupRule *schema.RollupRule | ||
) | ||
for _, mr := range ruleSet.MappingRules { | ||
latestSnapshot := mr.Snapshots[len(mr.Snapshots)-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check len(mr.Snapshots)
is nonzero first?
handlers/ruleset.go
Outdated
if mappingRule != nil && rollupRule != nil { | ||
return nil, nil, errMultipleMatches | ||
} | ||
return mappingRule, rollupRule, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return errNotFound
if both are nil?
handlers/ruleset.go
Outdated
return 0, nil, err | ||
} | ||
version := value.Version() | ||
ruleSet := &schema.RuleSet{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: more idiomatic to write var ruleSet schema.RuleSet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.