avocado icon indicating copy to clipboard operation
avocado copied to clipboard

HTML plugin: update JS template engines

Open amikan opened this issue 3 years ago • 8 comments

Minified JS and CSS files cannot be considered as real source files. That's why we should also add original Bootstrap, JQuery and DataTables sources. Because it is not easy to find original versions that were used to create minified ones, it's better to just bump to the latest.

I also need an assist of someone using HTML plugin in real test cases to proof this change is correct.

This patchset is required to pass lintian testing in Debian.

amikan avatar Oct 20 '21 15:10 amikan

This pull request introduces 4 alerts when merging 9b3e811b26f113be801184246af7998fc4aba807 into b896c1b4048960809ecb7d79b88784bd5b24008c - view on LGTM.com

new alerts:

  • 3 for Unsafe jQuery plugin
  • 1 for DOM text reinterpreted as HTML

lgtm-com[bot] avatar Oct 20 '21 15:10 lgtm-com[bot]

We need to have full version as 'source', and we need to have minified version for installing to target (for faster loading and caching). The second option is to have only full version and use some tool like 'minify' to create minified version during the build.

amikan avatar Oct 21 '21 06:10 amikan

We need to have full version as 'source', and we need to have minified version for installing to target (for faster loading and caching). The second option is to have only full version and use some tool like 'minify' to create minified version during the build.

Yes, I understand the usage. However keeping two files here seems strange to me. It is like versioning a binary file. The second option is more natural to me: We could build this during the installation process. However we need to keep in mind the effort and requirements to build it.

So, unless someone disagree here, maybe one solution would be to add the required tool to build this to requirements-dev.txt and build the minified version during the build/install process to avoid having both versioned. It is more work for now, but it will keep our repo cleaner and avoid double changes in the future.

beraldoleal avatar Oct 21 '21 12:10 beraldoleal

Hi @amikan Thanks for submitting this as a separate PR.

The files you want to add to avocado, are already packaged in Debian (package https://packages.debian.org/search?keywords=libjs-bootstrap), why don't you use these? Please, check in the archives of debian-mentors, there are good explanations on how to handle this issue: https://lists.debian.org/debian-mentors/2020/08/msg00086.html

If we're using some other javascript that's not packaged (I haven't checked)., then indeed update this PR

ana avatar Oct 21 '21 13:10 ana

Hi @amikan Thanks for submitting this as a separate PR.

The files you want to add to avocado, are already packaged in Debian (package https://packages.debian.org/search?keywords=libjs-bootstrap), why don't you use these? Please, check in the archives of debian-mentors, there are good explanations on how to handle this issue: https://lists.debian.org/debian-mentors/2020/08/msg00086.html

@ana what about the update per-se of the javascript files for general use? AFAICT the update in itself (without the dups) would be a good thing to have.

If we're using some other javascript that's not packaged (I haven't checked)., then indeed update this PR

clebergnu avatar Oct 21 '21 19:10 clebergnu

@ana Yes, it looks like there are no libs other then already shipped in libjs-bootstrap, libjs-jquery and libjs-jquery-datatables. But I'm afraid it can be an issue if running HTML plugin on web server because libs inside /usr/share/javascript/ will be not visible. What is the real use case of this plugin?

amikan avatar Oct 25 '21 07:10 amikan

@ana Yes, it looks like there are no libs other then already shipped in libjs-bootstrap, libjs-jquery and libjs-jquery-datatables. But I'm afraid it can be an issue if running HTML plugin on web server because libs inside /usr/share/javascript/ will be not visible. What is the real use case of this plugin?

Oh great, all the libraries are already shipped in Debian, then you can handle it in the packaging as mentioned above. About the use case, this plugin is used to create the HTML Avocado Job Report. For example, it's that JOB HTML : ~/avocado/job-results/job-2021-10-25T11.27-1557e47/results.html line you see when running avocado from the command line.

ana avatar Oct 25 '21 09:10 ana

Hi @amikan Thanks for submitting this as a separate PR. The files you want to add to avocado, are already packaged in Debian (package https://packages.debian.org/search?keywords=libjs-bootstrap), why don't you use these? Please, check in the archives of debian-mentors, there are good explanations on how to handle this issue: https://lists.debian.org/debian-mentors/2020/08/msg00086.html

@ana what about the update per-se of the javascript files for general use? AFAICT the update in itself (without the dups) would be a good thing to have.

Yeah indeed.

ana avatar Oct 25 '21 09:10 ana

Closing this based on the discussion in #5044

richtja avatar Jan 23 '24 10:01 richtja