flrs / visavail

A D3.js Time Data Availability Visualization
https://flrs.github.io/visavail/docs/samples/basic.html
MIT License
310 stars 59 forks source link

Percentage #45

Closed avsilverio closed 3 years ago

avsilverio commented 4 years ago

Add percentage functionality.

tanganellilore commented 4 years ago

Thanks for your pull!

One point, before accept any change on this library in terms of new features, please provide us some examples and tests of this. Describe also the objective af this features and method used to implement it.

After this we can accept your pull or we will integrate the features in our code Thanks.

avsilverio commented 4 years ago

I put an example in the samples directory and updated the index.html file.

tanganellilore commented 4 years ago

Hi @avsilverio ,

I have see your code, not in deep because I'm very busy. There are some points and function that for me is not correct but anyway I can rework it to aligne with te library funcionality.

What I don' understand in well is why we need to calcucalte the downtime or viceversa directly from library. My view is that the data used by library come from database or something like this and is pretty much easy calculate it from a database. Moreover if you consider that the library is pure javascript with usage of d3, why I need to demand the calculation of these parameter by library itself every times (consider that the function that you used is called every time that a graph is redraw, resize, update with a some impact on performance of library)?

What I can do is this: 1) I will rework your code to enable visavail to calculate downtime and so on in different way. This type of calculation will be performed only if a specific parameter on options is enabled like "calculate_downtime:enabled" 2) introduce a new key on dataset with downtime parameter, this can be used if you have already this information. With this all people that not need to calculate the parameter or prefer calculate himself have "0 impact " form performance point of view

If you agree I will work on it in in this and next week but I can't accept directly your pull request.

Are you agree?

Thanks for your support

avsilverio commented 4 years ago

Ok, but i put a condition to calculate downtime and uptime only if y_percentage is enabled. Thanks.

tanganellilore commented 4 years ago

Hi @avsilverio ,

now is better. I see your pull request, but you have used a "old" version of this library. you missing 2 commit from last release of library.

And is much simple for historical point of view have a the pull request with maximum 2-3 commits. Can you fix all of this?

avsilverio commented 4 years ago

Of course, thanks.

potapovnikita commented 3 years ago

When it will be merge?

tanganellilore commented 3 years ago

I'm waiting that @avsilverio align the code to the new released one.