search

mariusmuntean / ChartJs.Blazor

Brings Chart.js charts to Blazor
https://www.iheartblazor.com/
MIT License
691 stars 152 forks source link

Moved ArgValidation from ChartJs.Blazor.ChartJS.Common.Util to ChartJ… #94

Closed SeppPenner closed 4 years ago

SeppPenner commented 4 years ago

…s.Blazor.Util.

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

Joelius300 commented 4 years ago

Thank you, it looks good but I have one concern.

I just realized from this PR that ArgValidation is only used in the callback classes and only one method is not about callback stuff. These callback classes are all deleted in #70 and replaced entirely. Also why is ArgValidation public?! I actually think we don't need ArgValidation anymore 😅

I won't merge this PR because it would just cause unnecessary merge conflicts when #70 is done but thank you, you did exactly what I meant by the card. This is a good reminder for me to do something about ArgValidation in #70.

SeppPenner commented 4 years ago

I guess, you really need to finish #70 first before something else can be done 😄

No, it's ok. There needs a lot of stuff to be done in this project and since I have more time now due to the Corona stuff and home office now, I can again work on this project.

Joelius300 commented 4 years ago

Awesome, I'm happy you're here again :) Check what I wrote in the other closed PR just now, I appreciate the help.