-
Notifications
You must be signed in to change notification settings - Fork 209
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
Summarize results in test details tab (alternative) #3222
Summarize results in test details tab (alternative) #3222
Conversation
This is not complete, I think it requires update some code in the refreshInfoPanel |
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.
Besides the other comments, I also think that "stars" is generally a very unspecific name. These figures are already called "job module statistics" or similar in other places. So at least for being consistent it would be nice to stick to that name.
6657c58
to
dd0527c
Compare
Codecov Report
@@ Coverage Diff @@
## master #3222 +/- ##
==========================================
- Coverage 91.99% 91.98% -0.02%
==========================================
Files 211 211
Lines 12930 12933 +3
==========================================
+ Hits 11895 11896 +1
- Misses 1035 1037 +2
Continue to review full report at Codecov.
|
dd0527c
to
9dc771d
Compare
Fixed and squashed in two commits |
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.
Looks nice and elegant now. Thanks for working on this!
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.
Code looks fine.
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.
There's now redundant code (Perl implementation + JavaScript implementation). If that's the way to go I'm fine with it. However, it would be nice to add comments to each implementation stating that the other place possibly needs to be updated as well.
Automatic tests are missing. It is not that important for a feature like this but some basic test would likely easy to implement.
To help future uses, the code to paint the summary of a job has been extracted to a common helper.
https://progress.opensuse.org/issues/44654 This helps to get a quick overview in test details showing grouped results in tab on details page.
9dc771d
to
bfa192a
Compare
@Martchus Done |
https://progress.opensuse.org/issues/44654
This helps to get a quick overview in test details showing grouped results in tab on details page.
unlike in #3184 this approach used the backend to render the summary