-
Notifications
You must be signed in to change notification settings - Fork 38
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
Dynamic reports enhancement #111
Conversation
Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
9481491
to
3533bc6
Compare
3533bc6
to
631f84f
Compare
Signed-off-by: Pranav Gaikwad <pgaikwad@redhat.com>
631f84f
to
9b11c7f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is an excellent start and does a good job of capturing the high-level needs.
I think I have some questions on specifics if we are planning on coming back and updating this and creating either enhancements or prs of the underlying pieces, that also could make a ton of sense. just let me know and resolve the conversations :)
Issues have following properties: | ||
|
||
* Description: A text description in markdown format | ||
* Rule ID: Rule for which the issue was created |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to change this to RuleSet.RuleID?
* Version: Version of the dependency. | ||
* SHA: Checksum of the dependency. | ||
* Indirect: Whether the dependency is indirect. | ||
* Type: Type of the dependency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is type for here? can you add an example
|
||
#### Historical data | ||
|
||
Trends are generated from high level statistics of all analyses that happened so far. We want to store the computed statistics of every analysis and not the entire output. We want to limit the data points we store. But we are not defining a limit on the period of time for which the data stays. In this enhancement, we are saying all of the data from day 1 is available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we keeping every piece of data, OR are we saying it is only needed to keep the trend data say if it was calculated on each analysis run?
Personally, I would vote that we keep everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shawn-hurley I get what you're trying to say that we shouldn't be losing valuable data on purpose. But I also feel having too much data at user's disposal in the reports wouldn't be ideal from UX point of view. A good middle ground I feel would be - store most recent analysis data in Konveyor along with computed statistics, move raw data of previous runs to a secondary place but users don't see the raw old data in the report. This makes our life easy in getting the first iteration of reports out (where we dont care about old data), while not losing the old data in case we decide to do more intelligent things in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a high level that would be fine but does seem like we are agreeing that all data should be kept, just that it may not be available to the user through the UI. This makes sense to me can we more clearly codify that?
|
||
#### Application scope | ||
|
||
##### Dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to add the stretch goal of being able to see the whole graph? I assume this would change the data that is captured and reported in someway and I wonder if we need to make sure that we can upgrade to do this when we add it in the future?
|
||
#### Selection of applications | ||
|
||
Since there could be thousands of applications in the inventory, displaying all issues / dependencies for all applications in the inventory might not be ideal for user experience. In this enhancement, we are proposing that the users select a subset of applications via a separate filter for which they want to see the list of issues or dependencies. They can also select all applications, but we want that to be the choice of the user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a "may" or "must" select subset of applications?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shawn-hurley honestly, I don't know. @rromannissen any thoughts?
|
||
#### Filters on multiple columns | ||
|
||
One or more columns can be combined together to form complex searches. When filtering by multiple columns, logical AND between different types of columns is implied. Within the same type of columns, however, selecting multiple values implies a logical OR. For instance, when I select "mandatory" for the category column, and "java" and "golang" for target technology, it is implied that a list of issues with "mandatory" category AND with target technologies "java" OR "golang" will be returned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if an example URL that says the UI and Hub could comment on for this might be good?
Are we going to implement something like graphql or will we use regular query strings?
|
||
* As a user, I want to be able to see all tags generated for the application by the analyzer. | ||
|
||
* As a user, I want to be able to filter tags created by the analyzer by categories (tag types). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a user, I want to be able to see all tags generated for the application by the analyzer.
As a user, I want to be able to filter tags created by the analyzer by categories (tag types).
Is this different than existing tag display/filtering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jortel no. same as existing tag display
I assume that either this is out of date or we need to merge. I think we need to get this up to date and merge if we are cutting the alpha. |
@shawn-hurley not terribly out-of-date. Just need to move some things to future, as we decided to reduce the scope. Will update. |
688f13f
to
da4450e
Compare
@pranavgaikwad @shawn-hurley @jortel do you see further updates to this needed, or can we get an ACK and merge? |
@jwmatthews I dont see any further updates. |
No description provided.