Skip to content
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

[gui] Expandable runs #2953

Merged
merged 3 commits into from
Oct 30, 2020
Merged

[gui] Expandable runs #2953

merged 3 commits into from
Oct 30, 2020

Conversation

csordasmarton
Copy link
Contributor

No description provided.

tagName : t.versionTag,
title: t.versionTag,
tagName : t.versionTag ? t.versionTag : t.time,
title: t.versionTag ? t.versionTag : t.time,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may consider using this more concise syntax:

Suggested change
title: t.versionTag ? t.versionTag : t.time,
title: t.versionTag || t.time,


if run_history_filter and run_history_filter.tagIds:
query = query.filter(RunHistory.id.in_(run_history_filter.tagIds))
if stored.after:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stored.before and stored.after are not optional. Are these guaranteed to be 0 by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bruntib I tried it and the default value will be None if these fields are not set on the client side.

@csordasmarton csordasmarton force-pushed the expandable_runs branch 2 times, most recently from 5c21fbf to 2bef627 Compare October 12, 2020 08:18
@csordasmarton csordasmarton requested a review from bruntib October 12, 2020 08:19
Copy link
Contributor

@gyorb gyorb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you extend the history entries with some text to be more explicit about the numbers there.
So the users are not confused what those numbers mean (successful/failed analysis of the source files).
I think some users could think that those numbers are report numbers.

@csordasmarton
Copy link
Contributor Author

@gyorb I added a tooltip on these numbers and to this section.

I don't really want to add a string explicitly before the analyzer statistics section. By the way if I check a run line I can see that these are analysis statistics, so I can expect that here these are also analyzer statistics. But with my changes if I hover my mouse over this it will shows me this information too.

What do you think?

@csordasmarton csordasmarton requested a review from gyorb October 13, 2020 08:32
Copy link
Contributor

@gyorb gyorb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox breaks the tool tip lines a bit weird or is that appear only on my machine?
Like this:

Analysis statistics: shows the number of successfully analyzed
files
and the number of files which failed to analyze.

Chromium seems to be fine.

Otherwise LGTM.

@gyorb
Copy link
Contributor

gyorb commented Oct 29, 2020

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 51
- Added 128
           

Complexity increasing per file
==============================
- web/server/vue-cli/src/store/modules/run.js  9
- web/server/vue-cli/src/mixins/date.mixin.js  1
         

See the complete overview on Codacy

Copy link
Contributor

@gyorb gyorb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a space is missing

class="analyzer-statistics"
:title="'Analysis statistics: shows the number of successfully analyzed' +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:title="'Analysis statistics: shows the number of successfully analyzed' +
:title="'Analysis statistics: shows the number of successfully analyzed ' +

- Run items can be expanded which will show the run history entries.
- Add button to load more run history items.
@gyorb gyorb merged commit 1214c46 into Ericsson:master Oct 30, 2020
@csordasmarton csordasmarton deleted the expandable_runs branch November 6, 2020 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants