Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed try-catchs from MomentJsInterop.cs and ChartJsInterop.cs wher… #93

Closed
wants to merge 1 commit into from

Conversation

SeppPenner
Copy link
Contributor

…e the async-call inside the try is not awaited.

Fixes https://github.com/mariusmuntean/ChartJs.Blazor/projects/3#card-28660703.

…e the async-call inside the try is not awaited.
@Joelius300
Copy link
Contributor

Thanks for the PR.

This is exactly what I had in mind with that card but it's going to be a breaking change (since it also catches the expando stuff) so I can't merge this just now. Also this class is heavily affected by #70 so I would wait with adjusting things there until that's merged. Changes are high that I'll fix this in #70 anyway. Either way, I can't merge this PR but thank you, we can use it as a reference for later.

@Joelius300 Joelius300 closed this Mar 29, 2020
@SeppPenner
Copy link
Contributor Author

Ah, okay. I didn't know that this is affected by your rework there. I was just trying to start with the low hanging easy stuff before trying to break something here :)

@Joelius300
Copy link
Contributor

Joelius300 commented Mar 29, 2020

#70 is quite big, I know 😅 When there's not such a big change in the room, small things like this are greatly appreciated but right now might not be the best time for things like that.

If you want to help right now, I would greatly appreciate some opinions in #90 because I'm not sure how to handle that and once the discussion starts going it's easier to talk about it. Just saying though, if you don't have an opinion, don't force yourself to comment.
Also something that always helps is documenting what's wrong with current implementations. The scatter chart for example has many problems and searching for solutions to that, trying to find connections and common base classes, writing out what properties we need, collecting docs links etc is something that will have to be done at some point but requires some time and motivation. It's really not about code in that case, just writing out whats wrong and possible solutions is a great way to help us! And you don't even need code :)

@SeppPenner
Copy link
Contributor Author

I will check what I can do with the scatter chart :)

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