Skip to content

Conversation

@stweil
Copy link
Member

@stweil stweil commented Sep 2, 2024

It's not necessary to run CodeQL (and spend related resources like energy and time) for all kinds of file changes. Here changes of minimized JavaScript are ignored.

In addition some missing @Override tags are added to fix related CodeQL issues.

pull_request:
branches: [ master ]
paths-ignore:
- '**/*_min.js'
Copy link
Collaborator

@henning-gerhardt henning-gerhardt Sep 4, 2024

Choose a reason for hiding this comment

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

This will hide any file ending of "_min.js" which must not be necessary a minimized JavaScript version of a bigger JavaScript file. Other minimized JavaScript files (f.e Kitodo/src/main/webapp/js/jquery-2.1.1.min.js or Kitodo/src/main/webapp/WEB-INF/resources/js/libs/wavesurfer/wavesurfer.esm.js) which have a other name schema for minimization will be checked still. If I would to ignore a file I would name it directly and not use a global big match pattern. But this is only my opinion and can be ignored if they did not right.

Copy link
Member Author

@stweil stweil Sep 4, 2024

Choose a reason for hiding this comment

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

I think jquery-2.1.1.min.js should be removed because it is a ten years old security risk. Maybe Kitodo can use the JQuery package which is part of Debian / Ubuntu (libjs-jquery 3.6.1 in Debian stable) if this is required.

Copy link
Member

@solth solth May 5, 2025

Choose a reason for hiding this comment

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

I think @henning-gerhardt has a very good point here. The number of JavaScript files in Kitodo.Production is not great enough to justify the use of wildcard filters when explicitely listing each file that should be ignored would be more accurate and correct.

@stweil when you apply this change request and rebase your branch against the current main branch I think we can go forward and merge this pull request.

@stweil
Copy link
Member Author

stweil commented Sep 30, 2024

I created a separate pull request #6242 for the missing override annotations. Hopefully this makes it easier to get some CodeQL issues fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants