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

Run jscpd against the workspace #5041

Merged
merged 1 commit into from
Dec 24, 2023
Merged

Conversation

ferrarimarco
Copy link
Collaborator

@ferrarimarco ferrarimarco commented Dec 21, 2023

Proposed Changes

  • Run jscpd, gitleaks (not anymore, see Build the file list and run linters in parallel #5177), textlint (not anymore see Enable linting changed files with textlint #5100) against the entire workspace instead of
    running them over single files, one by one.
  • Implement a warning function for deprecated variables.
  • Deprecate the VALIDATE_JSCPD_ALL_CODEBASE variable.
  • Deprecate the EXPERIMENTAL_BATCH_WORKER variable, and related code, so we can implement a more scalable approach.
  • Remove duplicate configuration files from .github/linters when they are the same as the ones we ship in TEMPLATES.
  • Add a missing bad tests for ansible-lint.
  • Move ANSIBLE_DIRECTORY configuration when running tests in
    buildFileList, where similar configs are.
  • Simplify ansible-lint test cases to include only what's necessary, and
    not an entire set of roles, playbooks, and inventory.
  • Fix jscpd looks for code duplicate in images #3966

Readiness Checklist

In order to have this pull request merged, complete the following tasks.

Pull request author tasks

  • I included all the needed documentation for this change.
  • I provided the necessary tests.
  • I squashed all the commits into a single commit.
  • I followed the Conventional Commit v1.0.0 spec.

Super-linter maintainer tasks

  • Label as breaking if this change breaks compatibility with the previous released version.
  • Label as either: automation, bug, documentation, enhancement, infrastructure.

@ferrarimarco ferrarimarco added enhancement New feature or request O: backlog 🤖 Backlog, stale ignores this label labels Dec 21, 2023
@ferrarimarco ferrarimarco self-assigned this Dec 21, 2023
@ferrarimarco ferrarimarco changed the title feat: run jscpd, gitleaks against the workspace feat: run jscpd, gitleaks against the workspace Dec 21, 2023
@ferrarimarco ferrarimarco changed the title feat: run jscpd, gitleaks against the workspace Run jscpd, gitleaks, textlint against the workspace Dec 22, 2023
@ferrarimarco ferrarimarco force-pushed the lint-workspace-gitleaks-jscpd branch 9 times, most recently from 68b43b0 to 0e3b14f Compare December 22, 2023 16:32
@ferrarimarco ferrarimarco added the breaking big changes, potentially causes failure label Dec 22, 2023
@ferrarimarco ferrarimarco mentioned this pull request Dec 22, 2023
5 tasks
@ferrarimarco ferrarimarco force-pushed the lint-workspace-gitleaks-jscpd branch 7 times, most recently from f62738a to 46a6360 Compare December 22, 2023 20:23
@ferrarimarco
Copy link
Collaborator Author

(more context, originally posted in #4852 (comment))

This PR makes the following linters run against the entire workspace in one go, instead of linting file by file:

  • jscpd, because looking for duplicates in files processing them one by one doesn't consider duplicates in other files, to it makes this linter way less effective, and it costs a lot in terms of overall performance.
  • textlint, because it's based on plugins, and each plugin handles certain types of files. Also, textlint lets users specify custom extensions for a given file type in addition to the defaults. So, we let textlint handle this, instead of reimplementing the same logic. For example, with default plugins, textlint can lint txt and md files only, so there's no point in running it against all files one by one. If we/users install another textlint plugin, we don't need to modify any logic to handle an additional extension, nor custom extensions for any given plugin that users might have configured.
  • Gitleaks, because we want to scan the whole workspace anyway.

This approach also has the added benefit of letting users rely on the file listing and ignoring logic of each linter, so their configuration becomes more portable if they decide to run the same linter by other means.

@ferrarimarco ferrarimarco marked this pull request as ready for review December 23, 2023 18:26
- Run jscpd, gitleaks, textlint  against the entire workspace instead of
  running them over single files, one by one.
- Implement a warning function for deprecated variables.
- Deprecate the VALIDATE_JSCPD_ALL_CODEBASE variable.
- Remove duplicate configuration files when they are the same as the
  ones we provide in TEMPLATES.
- Add a missing tests for ansible-lint.
- Move ANSIBLE_DIRECTORY configuration when running tests in
  buildFileList, where similar configs are.
- Simplify ansible-lint test cases to include only what's necessary, and
  not an entire set of roles, playbooks, and inventory.
- Write instructions about major upgrades in the upgrade guide.
@ferrarimarco ferrarimarco merged commit 11b7010 into main Dec 24, 2023
6 checks passed
@ferrarimarco ferrarimarco deleted the lint-workspace-gitleaks-jscpd branch December 24, 2023 16:56
ferrarimarco added a commit that referenced this pull request Jan 6, 2024
textlint was expensive to run because we added every file in the list of
files to lint to FILE_ARRAY_NATURAL_LANGUAGE. In #5041, we mitigated
this issue but lost the ability to run textlint on changed files only.
Given that textlint ignore files for which it doesn't have a plugin
installed, and that we don't currently install additional plugins
besides the default ones to lint markdown files and text files, we let
textlint run on these files only, so we can have the feature to lint
only changed files with this linter as well, back.
ferrarimarco added a commit that referenced this pull request Jan 6, 2024
textlint was expensive to run because we added every file in the list of
files to lint to FILE_ARRAY_NATURAL_LANGUAGE. In #5041, we mitigated
this issue but lost the ability to run textlint on changed files only.
Given that textlint ignore files for which it doesn't have a plugin
installed, and that we don't currently install additional plugins
besides the default ones to lint markdown files and text files, we let
textlint run on these files only, so we can have the feature to lint
only changed files with this linter as well, back.
ferrarimarco added a commit that referenced this pull request Jan 9, 2024
textlint was expensive to run because we added every file in the list of
files to lint to FILE_ARRAY_NATURAL_LANGUAGE. In #5041, we mitigated
this issue but lost the ability to run textlint on changed files only.
Given that textlint ignore files for which it doesn't have a plugin
installed, and that we don't currently install additional plugins
besides the default ones to lint markdown files and text files, we let
textlint run on these files only, so we can have the feature to lint
only changed files with this linter as well, back.
ferrarimarco added a commit that referenced this pull request Jan 10, 2024
textlint was expensive to run because we added every file in the list of
files to lint to FILE_ARRAY_NATURAL_LANGUAGE. In #5041, we mitigated
this issue but lost the ability to run textlint on changed files only.
Given that textlint ignore files for which it doesn't have a plugin
installed, and that we don't currently install additional plugins
besides the default ones to lint markdown files and text files, we let
textlint run on these files only, so we can have the feature to lint
only changed files with this linter as well, back.
@ferrarimarco ferrarimarco changed the title Run jscpd, gitleaks, textlint against the workspace Run jscpd, gitleaks against the workspace Jan 15, 2024
@ferrarimarco ferrarimarco changed the title Run jscpd, gitleaks against the workspace Run jscpd against the workspace Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking big changes, potentially causes failure enhancement New feature or request O: backlog 🤖 Backlog, stale ignores this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jscpd looks for code duplicate in images
1 participant