How to deprecate griddle?

phetsims / griddle

Dynamic charting library built with Scenery

MIT License

2 stars 4 forks source link

How to deprecate griddle? #92

Closed samreid closed 3 years ago

samreid commented 3 years ago

We created a new charting library "bamboo" that corrects many of griddle's deficiencies. How/when should be deprecate griddle? Some possibilities:

[x] Add documentation to README.md
[x] Add documentation to the deprecated files
[x] add assert && deprecationWarning(...) to the affected constructors

Should all parts of griddle be deprecated? The main files that seem like they should not be deprecated are: SpanNode and GridNode. Also, should we move non-deprecated things elsewhere? For example, GridNode could move to scenery-phet?

I'm happy to work on these steps, but I want to touch base with @pixelzoom before starting.

pixelzoom commented 3 years ago

For the deprecated classes, we should do what PhET has done elsewhere (SimpleDragHandler, etc.):

[ ] add @deprecated annotations, so that they are flagged in WebStorm
[ ] add assert && deprecationWarning(...) so that they are noted during code review

Re GridNode.... I'm not a fan. The same thing can be done with bamboo, and much more cleanly.

Re SpanNode... I'm not familiar with this. Is it useful in the bamboo framework? If so, move it to bamboo, or write a version of it for bamboo.

Everything else in griddle I would deprecate.

samreid commented 3 years ago

@jessegreenberg and I reviewed this today, in particular about GridNode API and about deprecating things that don't necessarily have a replacement at the moment, such as the chart cursor, and grid node.

We identified a bamboo bug where gridlines can get out of bounds, I'll create a new issue for that

We agreed it seems reasonable to mark griddle components as deprecated, even though bamboo is more about building blocks and not so much about high-level APIs (yet?). For instance, even though it is not convenient to create a grid node in the play area using bamboo components, we prefer the transform-based API defined by bamboo's ChartModel.

There are no plans to rewrite anything that is currently using griddle.

samreid commented 3 years ago

I added deprecation warnings and JSDoc. I did not update the README, because it may be auto-generated. All remaining work tracked elsewhere, closing.