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

build: add MegaLinter config files #1960

Closed
wants to merge 3 commits into from
Closed

Conversation

eitsupi
Copy link
Member

@eitsupi eitsupi commented Feb 25, 2023

Close #1944

  • Automatic fixes are disabled to avoid conflicts with existing pre-commits.
  • The linters for languages (Rust, JS, and Python) which are already configured (?) are disabled.

@eitsupi eitsupi reopened this Feb 25, 2023
@max-sixty
Copy link
Member

I had a look through the checks. Some are quite strict, in a way that is actually quite helpful, but we wouldn't want to completely block code — for example:

  Clone found (rust):
   - prql-compiler/tests/integration/main.rs [178:25 - 193:9] (15 lines, 108 tokens)
     prql-compiler/tests/integration/main.rs [103:25 - 118:7]

In order to fully test it, we might want to open a branch on this repo — it's trying to do things like add comments to the PR, and failing because it's on a fork. We could get around that later by having it run on pull-request-target, which gives more permissions but runs based on the code on main, not the proposed PR.

@eitsupi you should have permissions to push a branch to this repo.


I don't see it making changes to Java code etc?

For me, there are two helpful things from these:

  • Autofixing things that don't change semantics, like formatting
  • Giving information about potential bad code, but not stopping merging — semantics are up to the contributor, and we shouldn't slow anything down

One advantage of this was to run formatters on code we don't use much, like Java & C (and maybe something like python, which hardly exists in the repo). But maybe those don't actually have formatters?

@eitsupi
Copy link
Member Author

eitsupi commented Feb 26, 2023

@max-sixty Thanks for taking a look at this.
You are right, I probably should have pushed the branch to this repository instead of my fork...
To see how comments work, please see here as I opened a PR from the same branch on the fork. eitsupi#2

I don't see it making changes to Java code etc?

So far only the linter corresponding to the fixes included in the PR is working.
We could have configured it to inspect the entire project, but I forgot how to do it so I will check the documentation later to demonstrate that......

But maybe those don't actually have formatters?

While the ability exists for some linters or formatters to automatically correct, but auto-fixes are completely stopped to avoid conflicts with pre-commits CI.
Users will need to manually fix them.

If we want MegaLinter to automatically fix your problem, we need to stop the pre-commit auto-fix.
(I am fine with either. I have no strong opinion.)

I had a look through the checks. Some are quite strict, in a way that is actually quite helpful, but we wouldn't want to completely block code — for example:

We need to determine which Linter to disable, or to generate a warning but not treat it as an error.....
The initial setup is quite labor intensive because of the need to look at the settings for a large number of linters.
(This is why I have not been able to implement MegaLinter in most of my projects.)

@eitsupi eitsupi closed this Feb 26, 2023
@eitsupi eitsupi reopened this Feb 26, 2023
@eitsupi
Copy link
Member Author

eitsupi commented Feb 26, 2023

@max-sixty Could you please tell me why pre-commit CI is failing in this PR?
Sorry, but I can't make sense of the error message.

@max-sixty
Copy link
Member

@max-sixty Could you please tell me why pre-commit CI is failing in this PR? Sorry, but I can't make sense of the error message.

It's not permitted to fix workflow files by GitHub.

(at the bottom it states "GitHub prevented pre-commit.ci from autofixing this pr due to autofixes to a workflow file")

So we need to run locally and push the result. Lmk if you don't have it installed and I'm happy to do it

@eitsupi
Copy link
Member Author

eitsupi commented Feb 27, 2023

The last commit made all files subject to Lint and found that it takes more than 5000 seconds (!) to check CSS.
Clearly this Linter needs to be disabled.

+----SUMMARY----+--------------------------+---------------+-------+-------+--------+--------------+
| Descriptor    | Linter                   | Mode          | Files | Fixed | Errors | Elapsed time |
+---------------+--------------------------+---------------+-------+-------+--------+--------------+
| ❌ ACTION     | actionlint               | list_of_files |    16 |       |      2 |        0.19s |
| ✅ BASH       | bash-exec                | file          |     2 |       |      0 |        0.01s |
| ❌ BASH       | shellcheck               | list_of_files |     2 |       |      1 |        0.06s |
| ◬ BASH        | shfmt                    | list_of_files |     2 |       |      1 |        0.03s |
| ❌ C          | cpplint                  | list_of_files |     2 |       |      6 |        0.21s |
| ◬ COPYPASTE   | jscpd                    | project       |   n/a |       |     11 |       17.46s |
| ❌ CPP        | cpplint                  | list_of_files |     1 |       |      2 |        0.14s |
| ◬ CSHARP      | csharpier                | list_of_files |     4 |       |      1 |        1.57s |
| ◬ CSHARP      | dotnet-format            | file          |     4 |       |      4 |        2.79s |
| ❌ CSS        | stylelint                | list_of_files |    12 |       |      1 |     5138.48s |
| ❌ DOCKERFILE | hadolint                 | list_of_files |     1 |       |      1 |        0.09s |
| ❌ HTML       | djlint                   | list_of_files |    19 |       |     16 |        1.07s |
| ❌ HTML       | htmlhint                 | list_of_files |    19 |       |     75 |        0.55s |
| ❌ JAVA       | checkstyle               | list_of_files |     4 |       |     76 |        3.09s |
| ❌ JAVA       | pmd                      | list_of_files |     4 |       |      1 |        3.17s |
| ✅ JSON       | eslint-plugin-jsonc      | list_of_files |     9 |       |      0 |        3.07s |
| ❌ JSON       | jsonlint                 | list_of_files |     9 |       |      1 |        0.27s |
| ✅ JSON       | prettier                 | list_of_files |     9 |       |      0 |         1.7s |
| ✅ JSON       | v8r                      | list_of_files |     9 |       |      0 |       10.87s |
| ❌ MAKEFILE   | checkmake                | file          |     1 |       |      1 |         0.0s |
| ◬ MARKDOWN    | markdownlint             | list_of_files |    97 |       |     48 |        1.04s |
| ✅ MARKDOWN   | markdown-link-check      | list_of_files |    97 |       |      0 |       53.58s |
| ◬ MARKDOWN    | markdown-table-formatter | list_of_files |    97 |       |      1 |        0.38s |
| ❌ PHP        | phpcs                    | list_of_files |     3 |       |      1 |        0.15s |
| ✅ PHP        | phplint                  | list_of_files |     3 |       |      0 |        0.09s |
| ❌ PHP        | phpstan                  | list_of_files |     3 |       |      9 |        2.48s |
| ❌ PHP        | psalm                    | list_of_files |     3 |       |     11 |        0.62s |
| ◬ REPOSITORY  | checkov                  | project       |   n/a |       |      6 |       16.52s |
| ◬ REPOSITORY  | devskim                  | project       |   n/a |       |      3 |        1.23s |
| ✅ REPOSITORY | dustilock                | project       |   n/a |       |      0 |        1.28s |
| ✅ REPOSITORY | gitleaks                 | project       |   n/a |       |      0 |        3.75s |
| ✅ REPOSITORY | git_diff                 | project       |   n/a |       |      0 |        0.05s |
| ✅ REPOSITORY | secretlint               | project       |   n/a |       |      0 |         3.3s |
| ✅ REPOSITORY | syft                     | project       |   n/a |       |      0 |        2.17s |
| ◬ REPOSITORY  | trivy                    | project       |   n/a |       |      1 |        9.96s |
| ❌ SPELL      | misspell                 | list_of_files |   201 |       |      1 |        0.35s |
| ✅ SQL        | sql-lint                 | file          |     1 |       |      0 |        0.38s |
| ❌ SQL        | tsqllint                 | list_of_files |     1 |       |      1 |         0.5s |
| ✅ XML        | xmllint                  | list_of_files |     1 |       |      0 |         0.0s |
| ◬ YAML        | prettier                 | list_of_files |    30 |       |      1 |        1.74s |
| ❌ YAML       | v8r                      | list_of_files |    30 |       |      1 |       36.88s |
| ✅ YAML       | yamllint                 | list_of_files |    30 |       |      0 |        0.71s |
+---------------+--------------------------+---------------+-------+-------+--------+--------------+

Open another PR (#1974) and continue this.

@eitsupi eitsupi closed this Feb 27, 2023
@eitsupi eitsupi deleted the megalinter branch February 27, 2023 10:36
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.

Use MegaLinter?
2 participants