Add initial configuration for ESLint (fix Codacy issues)

[stweil]

Codacy uses ESLint. Tell it that certain variables like PF are defined. This avoids related issue reports about undefined variables.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[stweil]

It looks like this PR has not the desired effect on Codacy. Therefore I marked it as draft now

[stweil]

Related documentation:

• https://docs.codacy.com/codacy-guardrails/codacy-guardrails-how-to-configure-rules/

[stweil]

According to the Codacy documentation, configuration files are always read from the default branch. So the change here has no effect until it was merged into the main branch. I suggest to do this, then we will see whether it works.

[henning-gerhardt]

I can not review this as

• I did not have access to nor I'm using Codacy
• I did not know how Codacy is set up for Kitodo.Production and so I don't know is this change is really working or did anything
• I'm not familiar with ESLint and did not find any documentation of all this used configuration parameters
• ESLint is using be default a different configuration file naming

[stweil]

I did not have access to nor I'm using Codacy

https://app.codacy.com/gh/kitodo/kitodo-production/issues/current does not work for you?

I did not know how Codacy is set up for Kitodo.Production

The Codacy Static Code Analysis is part of the checks which run for each pull request here.

[stweil]

ESLint is using be default a different configuration file naming

Which one? Please compare this documention.

[solth]

According to the Codacy documentation, configuration files are always read from the default branch. So the change here has no effect until it was merged into the main branch. I suggest to do this, then we will see whether it works.

Then you should first merge it into the main branch of your fork (or temporarily set your branch eslint as the default branch of your fork) to verify whether the file has the intended effect or not before opening a pull request.

[stweil]

Arved, this is often a good idea. In situations where this is possible I test it exactly like this. But here we have an external action, not a simple GitHub action which can be run on any fork. I cannot add my fork of kitodo-production to Codacy. I tried it, but it simply does not get added. Therefore I cannot run the proposed test.

I now managed to add Codacy to my personal fork. With an updated version of .eslintrc.json, the warnings for undefined PF were fixed. The same updated file also triggers lots of new warnings. Some of these might be important and desired, others less interesting.

So the new file works, but we have to decide which kinds of issues should be reported or not reported by Codacy / ESLint. It is possible to add exclude_paths in a new file .codacy.yml for generated code, test directories, and minified JS that shouldn't be linted.

Of course it would also be possible to remove the Codacy app in the settings of kitodo/kitodo-production. Then it would no longer check pull requests or report issues. I don't suggest this.

[solth]

@stweil I agree that we should not deactivate Codacy entirely. I like the idea of excluding certain paths from the check. Perhaps the results also be restricted by severity?

[stweil]

Sure, fine tuning might be possible (I'd have to look it up in the documention to see what is possible).

I suggest to merge this PR here. Then we will see what gets reported in the future, and we can decide which kind of issues we want to ignore. Filtering purely on the severity would in my opinion not be sufficient. For example bad style would not be a high severity, but we want to avoid it nevertheless.

[solth]

We don't use Mojarra as JSF implementation, but rather MyFaces, so why do we need to define "mojarra" as a global here? Does it occur somewhere in the code?

[solth]

We have to keep an eye on this, because there may be other false positives apart from "PF" etc. when marking supposedly undefined symbols.