Skip to content

Conversation

@rg911
Copy link
Contributor

@rg911 rg911 commented Mar 10, 2020

Copy link
Contributor

@fboucquez fboucquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users and model objects don't use IdGenerator directly. They go through NamespaceMosaicIdGenerator (for example NamespaceId).

The max depth default param value hides it from the main model objects so they ended up using the default value.

We can either:

  1. Remove the default value and propagate maxDepth to the client objects.
  2. Remove that validation to keep the API clean. Maybe provide a validate method after the namespace is created.

I don't particularly like either, do you have another option?

@rg911
Copy link
Contributor Author

rg911 commented Mar 11, 2020

Users and model objects don't use IdGenerator directly. They go through NamespaceMosaicIdGenerator (for example NamespaceId).

The max depth default param value hides it from the main model objects so they ended up using the default value.

We can either:

  1. Remove the default value and propagate maxDepth to the client objects.
  2. Remove that validation to keep the API clean. Maybe provide a validate method after the namespace is created.

I don't particularly like either, do you have another option?

Discussed with @dgarcia360
maxDepth was hardcoded on the server previously but now made configurable. I have removed it and the validation to let the server validate the depth.

@rg911 rg911 added the story-1 label Mar 11, 2020
@rg911 rg911 added this to the 0.9.3.1 enhancement milestone Mar 11, 2020
@rg911 rg911 merged commit e2a694f into symbol:master Mar 11, 2020
@fboucquez fboucquez deleted the task/g455_namespace_max_depth branch April 13, 2020 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

namespace_max_depth should not be hardcoded

2 participants