Page MenuHomeSoftware Heritage
Differential D3293

scanner: dashboard file visualization per directory path
ClosedPublic
Actions

Authored by DanSeraf on Jun 16 2020, 6:31 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDTSCN0f10ec6ae8fe: dashboard: file visualization per directory path
Summary

Dashboard file browser (Related T2364)

Display the files relative to the directory path selected in the sunburst chart.

Output example:

Diff Detail

Repository
rDTSCN Code scanner
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

DanSeraf created this revision.Jun 16 2020, 6:31 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJun 16 2020, 6:31 PM
swh-public-ci added a comment.Jun 16 2020, 6:34 PM

Build is green

Patch application report for D3293 (id=11673)

Rebasing onto bf9e586436...

Current branch diff-target is up to date.
Changes applied before test
commit 6aa1c86f3cb64a79e230fa312b7b8c71d22fb38c
Author: Daniele Serafini <me@danieleserafini.eu>
Date:   Tue Jun 16 18:24:56 2020 +0200

    dashboard: file visualization per directory path

See https://jenkins.softwareheritage.org/job/DTSCN/job/tests-on-diff/30/ for more details.

Harbormaster completed remote builds in B12881: Diff 11673.Jun 16 2020, 6:34 PM
DanSeraf edited the summary of this revision. (Show Details)Jun 16 2020, 6:45 PM
vlorentz added subscribers: zack, vlorentz.Jun 16 2020, 9:37 PM

Two quick comments, I'll do an actual review on thursday:

  • I don't think adding an external dependency (the .css) is a good idea (it might change or disappear). But this file doesn't seem to have a license so we probably can't bundle it either (ping @zack )
  • It's missing tests. I understand it's not easy to do for a GUI, but could you see what you can do about it?
zack added a comment.Jun 17 2020, 9:12 AM
In D3293#80514, @vlorentz wrote:
  • I don't think adding an external dependency (the .css) is a good idea (it might change or disappear). But this file doesn't seem to have a license so we probably can't bundle it either (ping @zack )

Yeah, agreed.

To use that CSS we should either incorporate it (incurring in the copyright and/or license problem mentioned by @vlorentz), or hotlink to it (incurring in tracking/404 problems). Neither of which is great.

We should either find another CSS which is FOSS-licensed, or roll our own—the former option being preferable.

DanSeraf updated this revision to Diff 11758.Jun 19 2020, 4:41 PM
  • css as static asset
swh-public-ci added a comment.Jun 19 2020, 4:45 PM

Build has FAILED

Patch application report for D3293 (id=11758)

Rebasing onto bf9e586436...

Current branch diff-target is up to date.
Changes applied before test
commit 536bb0fca288651365c594165a1237f736b0a5f2
Author: Daniele Serafini <me@danieleserafini.eu>
Date:   Tue Jun 16 18:24:56 2020 +0200

    dashboard: file visualization per directory path

Link to build: https://jenkins.softwareheritage.org/job/DTSCN/job/tests-on-diff/31/
See console output for more information: https://jenkins.softwareheritage.org/job/DTSCN/job/tests-on-diff/31/console

Harbormaster failed remote builds in B12968: Diff 11758!Jun 19 2020, 4:45 PM
DanSeraf updated this revision to Diff 11759.Jun 19 2020, 4:51 PM
  • init.py in dashboard directory
swh-public-ci added a comment.Jun 19 2020, 4:54 PM

Build is green

Patch application report for D3293 (id=11759)

Rebasing onto bf9e586436...

Current branch diff-target is up to date.
Changes applied before test
commit 8c28d2614893b44e6f7bca81ab71a7274bd90395
Author: Daniele Serafini <me@danieleserafini.eu>
Date:   Tue Jun 16 18:24:56 2020 +0200

    dashboard: file visualization per directory path

See https://jenkins.softwareheritage.org/job/DTSCN/job/tests-on-diff/32/ for more details.

Harbormaster completed remote builds in B12969: Diff 11759.Jun 19 2020, 4:54 PM
DanSeraf added a comment.Jun 19 2020, 4:54 PM
In D3293#80514, @vlorentz wrote:
  • It's missing tests. I understand it's not easy to do for a GUI, but could you see what you can do about it?

Regarding the tests, i don't know if testing the length of the returning table could be useful.
Unfortunately the expected values can't be tested.

vlorentz added a comment.Jun 19 2020, 5:23 PM
In D3293#81209, @DanSeraf wrote:

Unfortunately the expected values can't be tested.

Why not?

I must be missing something; how is the new .css file included from the html?

DanSeraf added a comment.Jun 20 2020, 4:37 PM
In D3293#81217, @vlorentz wrote:
In D3293#81209, @DanSeraf wrote:

Unfortunately the expected values can't be tested.

Why not?

Because the dash_html_components checks for object identity only. I wrote a comment inside the test.

I must be missing something; how is the new .css file included from the html?

Since the dash_boostrap_components uses Bootstrap as CSS, it's better to use it also to manage the columns width.

DanSeraf updated this revision to Diff 11772.Jun 20 2020, 4:37 PM
  • workaround to check the table body
  • bootstrap css update
swh-public-ci added a comment.Jun 20 2020, 4:40 PM

Build is green

Patch application report for D3293 (id=11772)

Rebasing onto bf9e586436...

Current branch diff-target is up to date.
Changes applied before test
commit a2efb5490b09a8433773c8ad43b8021077dcb19a
Author: Daniele Serafini <me@danieleserafini.eu>
Date:   Tue Jun 16 18:24:56 2020 +0200

    dashboard: file visualization per directory path

See https://jenkins.softwareheritage.org/job/DTSCN/job/tests-on-diff/33/ for more details.

Harbormaster completed remote builds in B12982: Diff 11772.Jun 20 2020, 4:40 PM
vlorentz added a comment.Jun 22 2020, 10:19 AM

@zack How do you feel about the minified bootstrap code?

zack added a comment.Jun 22 2020, 10:31 AM
In D3293#81350, @vlorentz wrote:

@zack How do you feel about the minified bootstrap code?

Ideally we should have both the minified and non-minified version in Git.

(Even better would be generating the minified from the non-minified at deploy time, but that will inflate the deployment dependencies by a lot, so I don't think it's worth it for something that is not primarily a web app.)

DanSeraf updated this revision to Diff 11798.Jun 22 2020, 2:50 PM
  • non-minified css in assets
swh-public-ci added a comment.Jun 22 2020, 2:52 PM

Build is green

Patch application report for D3293 (id=11798)

Rebasing onto bf9e586436...

Current branch diff-target is up to date.
Changes applied before test
commit 9b1c7bf4109135d933aa83eee913d687b7a45911
Author: Daniele Serafini <me@danieleserafini.eu>
Date:   Tue Jun 16 18:24:56 2020 +0200

    dashboard: file visualization per directory path

See https://jenkins.softwareheritage.org/job/DTSCN/job/tests-on-diff/34/ for more details.

Harbormaster completed remote builds in B13010: Diff 11798.Jun 22 2020, 2:52 PM
vlorentz accepted this revision.Jun 22 2020, 7:21 PM
This revision is now accepted and ready to land.Jun 22 2020, 7:21 PM
Closed by commit rDTSCN0f10ec6ae8fe: dashboard: file visualization per directory path (authored by DanSeraf). · Explain WhyJun 22 2020, 7:39 PM
This revision was automatically updated to reflect the committed changes.
DanSeraf added a commit: rDTSCN0f10ec6ae8fe: dashboard: file visualization per directory path.

Revision Contents
Changeset List

PathSize
5 lines
1 line
swh/
scanner/
2 lines
dashboard/
assets/
9729 lines
7 lines
102 lines
tests/
51 lines

Diff 11823

View Options

mypy.ini

Loading...
View Options

requirements.txt

Loading...
View Options

swh/scanner/cli.py

Loading...
View Options

swh/scanner/dashboard.py

Loading...
View Options

swh/scanner/dashboard/__init__.py

Loading...
View Options

swh/scanner/dashboard/assets/bootstrap.css

Loading...
View Options

swh/scanner/dashboard/assets/bootstrap.min.css

Loading...
View Options

swh/scanner/dashboard/dashboard.py

Loading...
View Options

swh/scanner/tests/test_dashboard.py

Loading...
About · Developer information · Legal