[PATCH] D111289: [LNT] Refactored the Graph page to use the library plotly instead of jQuery.flot

Mon Oct 25 14:34:21 PDT 2021

slydiman added a comment.

In D111289#3085371 <https://reviews.llvm.org/D111289#3085371>, @cmatthews wrote:

> As a start, split the non-patch related changes out. Specifically, at least split the changes in: test_matrix_page.py, test_api.py, the renames, the error message updates in views.py, the refactoring in api.py.

Note Plotly supports datetime and series for X-axis. Both types are implemented here. Refactoring in api.py and such is necessary to process the data correctly. Note some graph data is prepared during the page rendering and the rest graph data is requested by JS from the browser via api. So api.py must be updated. test_api.py depends on api.py

Old behavior: revisions 1.0, 2.3 and 45.67 were placed on X-axis disproportionately. 1.0 and 2.3 stick together, but the distance between 2.3 and 45.67 is too long.

New behavior: everything is located on the X-axis in proportion as series. Revisions may not contain digits at all. And https://reviews.llvm.org/D109577 looks reasonable after this patch.

> We need test coverage in all this code being added as well.

What code being added do you mean? 
lnt_regression.js is just a copy of the old version of lnt_graph.js
The code for jQuery.Flot has been replaced with the code for Plotly.
All tests are the same, but some tests were updated.
Some code from Graph and Matrix has been combined in few helper functions like load_plot_parameter(). Renaming graph_parameters and data_parameters to plot_parameters is a part of it.

Right, it is possible to separate error messages and do not touch test_matrix_page.py for now (test_matrix_page.py depends on error messages.). But the half of error messages is in the new code. I don't see a reason for moving the half of error messages to a separate patch.

> Move some of this out into a series of refactorings, before the main final change, that will help make it simpler to reason about the changes.

The final goal is the quality and stable code, right? It seems the new implementation is already tested and worked well. It is very easy to break something trying to split it into a series of refactorings.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111289/new/

https://reviews.llvm.org/D111289