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

Move Report Viewer to vite #1029

Merged
merged 24 commits into from
Apr 19, 2023
Merged

Move Report Viewer to vite #1029

merged 24 commits into from
Apr 19, 2023

Conversation

Kr0nox
Copy link
Member

@Kr0nox Kr0nox commented Apr 14, 2023

This PR moves the Report Viewer to vite. This includes

  • Movint to vite
  • Replacing vuex with pinia
  • Removing legacy node dependencies
  • Adding husky for pre-commit linting
  • Adding new testing templates
  • Cleaning up the package.json

@Kr0nox
Copy link
Member Author

Kr0nox commented Apr 14, 2023

@sebinside Could you test the report viewer for any bugs and issues with a few larger and real samples?

@tsaglam tsaglam added enhancement Issue/PR that involves features, improvements and other changes major Major issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies labels Apr 17, 2023
Copy link
Contributor

@TwoOfTwelve TwoOfTwelve left a comment

Choose a reason for hiding this comment

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

Code looks good,
but I tried running it and when trying to load a report the error "Invalid JSON: [object Object],[object Object]" keeps poping up.

Also I think the README.md in /report-viewer ist not up to date anymore as the command "npm run serve" does not seem to be working.

@TwoOfTwelve TwoOfTwelve self-requested a review April 17, 2023 10:59
Copy link
Contributor

@TwoOfTwelve TwoOfTwelve left a comment

Choose a reason for hiding this comment

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

My mistake, I used an invalid file.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

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

First of all, good work, looks promising and is already near to being merged. I had a look at your changes and also tested the report viewer with real-world data consisting of 20,000 comparisons. Here are my findings:

✅ Report Viewer behaves as expected for common operations
✅ Using the zip drag n drop method works in development
✅ Using the zip drag n drop method works after build using vite preview
❌ Using the src/files method with the extracted content of the zip file works neither in dev nor after build. The XMLHttpRequest gets printed to the console together with the warning Unable to build comparison file. This behavior remains when using a different local server like npm http-server.

Please have a look at this remaining bug. Afterward, we can merge this PR. Thank you!

@sebinside sebinside merged commit 64d1436 into develop Apr 19, 2023
@sebinside sebinside deleted the report-viewer-vite branch April 19, 2023 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR that involves features, improvements and other changes major Major issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants