Native review for suzuki-shin/elm-chartjs about package.elm-lang.org HOT 6 CLOSED

Hey, I just asked the mailing list to help with this review, so I hope they will take a first pass while I am traveling. In the meantime, I'd let folks know about the library on the list so people can use it via github.

from package.elm-lang.org.

Thank you.

from package.elm-lang.org.

Just reading the Elm code, I notice there are many types with similar names. We have Chart and ChartObj, despite the JS library not making the distinction. Then there's DataType1 and DataType2, as well as DatasetType1 and 2 which aren't exposed but need to be to use the library. These types have helper functions which are unexplained. There is very little documentation.

I realize that it can be hard to translate duck-typed JS libraries into Elm. Better names and documentation will go a long way. If Suzuki needs help with the English, I'm sure someone will volunteer.

update and attachOn appear to be impure functions.

from package.elm-lang.org.

I thank you for your review.

As you say, ChartObj did not need to think now.
Chart.js has 6 types of chart (Line, Bar, Radar, PolarArea, Pie, Doughnut). And among them, Line, Bar, Radar will receive the data of the type DataType1, and PolarArea, Pie, Doughnut will receive the data of type DataType2. Because these good name put did not think of, I have to name a DataType1,2 like. Please tell me if there is the idea of a better name.
DatasetType1 and DatasetType2 is a component of each DataType1 and DataType2. DatasetType1 is an element of datasets field of DataType1.
I will write a description of the helper functions.

I so English is not good, I am glad and get to point out if there is a strange place.

Do you think that it is good to how for attachOn and update?

In a few days, I will trying to fix and comment here.

from package.elm-lang.org.

The native review pipeline is stalled. That's probably not news to you, but I wanted to leave a note on all the open reviews. I cannot speak for everyone, but as one of the reviewers I just want to speak up and say that I will not be doing more reviews. The warning about publishing native packages has changed, and Evan has mentioned multiple times in different places that one of his priorities for 0.17 is improving this process. So I'm going to wait for that. I'm sorry we cannot move forward with publishing your library, but my experience is that even if we review and approve it, it will become stuck waiting for Evan to whitelist it.

from package.elm-lang.org.

Sorry to keep this in limbo for so long. I am moving towards the plan outlined here and discussed a bit further in the 0.17 announcement. The right thing to for now is use ports or get advice on the mailing list or on slack.

from package.elm-lang.org.