Skip to content

Reorganized handling of CellType.name - #2142

Merged
lossyrob merged 3 commits into
locationtech:masterfrom
metasim:celltype-name
Apr 20, 2017
Merged

lossyrob merged 3 commits into
locationtech:masterfrom
metasim:celltype-name

Conversation

@metasim

@metasim metasim commented Apr 18, 2017

Copy link
Copy Markdown
Member

to make it the authoritative representation of cell type, over toString.
Moved name resolution to companion object to make it symmetric with fromName implementation.

In the previous implementation, inheritance was used as the means of having different values of name per type. That caused problems when trying to come up with a general solution based on calling super to generate derivatives, since self-types were used over direct inheritance. Having the to/from name handling in the companion also keeps coupled implementations closer to each other.

cc: @moradology

Signed-off-by: Simeon H.K. fitch [email protected]

metasim added 2 commits April 18, 2017 15:22
to make it the authoritave representation of cell type, over `toString`.
Moved `name` resolution to companion object to make it symmetric with `fromName` implementation.

Signed-off-by: Simeon H.K. fitch <[email protected]>

@lossyrob lossyrob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 after deprecation string update

* @param name A string representing a cell type, as reported by [[DataType.name]] e.g. "uint32"
* @return The CellType corresponding to `name`
*/
@deprecated("Use `fromName` instead", "1.2.0")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you change to 1.1.0?

@lossyrob lossyrob added this to the 1.1 milestone Apr 19, 2017
@lossyrob
lossyrob merged commit bd30bec into locationtech:master Apr 20, 2017
@metasim
metasim deleted the celltype-name branch April 20, 2017 16:33
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.

2 participants