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

Don't forcefully validate Git repos if not needed #4953

Merged
merged 15 commits into from
Dec 7, 2023
Merged

Conversation

ferrarimarco
Copy link
Collaborator

@ferrarimarco ferrarimarco commented Dec 6, 2023

Fixes #4952

Proposed Changes

  1. Add a test case to run super-linter against a subdirectory after setting USE_FIND_ALGORITHM=true.
  2. Move the configuration of git safe.directory after we set GITHUB_WORKSPACE, and not before because GITHUB_WORKSPACE might be initialized when running locally.
  3. Validate variables before validating the Git repository, so that we get those variables initialized correctly before using them.
  4. Validate the Git directory regardless of the value of VALIDATE_ALL_CODEBASE because we need a valid Git repository and references both when VALIDATE_ALL_CODEBASE=true and when VALIDATE_ALL_CODEBASE=false.
  5. Validate the Git directory when USE_FIND_ALGORITHM == false, not when USE_FIND_ALGORITHM != false.
  6. Initialize GITHUB_SHA when running locally and when USE_FIND_ALGORITHM == false.
  7. Fail with a fatal error if we can't add the workspace to the list of safe Git directories.
  8. Fix log level color markers for non-default cases.
  9. Set DEFAULT_BRANCH when running tests.

Readiness Checklist

Author/Contributor

  • I included all the needed documentation for this change.
  • I provided the necessary tests.

Reviewing Maintainer

  • Label as breaking if this is a large, fundamental change.
  • Label as either: automation, bug, documentation, enhancement, infrastructure.

BEGIN_COMMIT_OVERRIDE
fix: don't forcefully validate Git repos if not needed (#4953)
END_COMMIT_OVERRIDE

@rmatte
Copy link

rmatte commented Dec 6, 2023

When I look at the commit for this it only appears to include the code for "2. Add a workflow step to catch this issue."

@ferrarimarco
Copy link
Collaborator Author

When I look at the commit for this it only appears to include the code for "2. Add a workflow step to catch this issue."

Yep. As I explained in #4952, I'd like to see a failing workflow, that we'll fix by committing the change you suggested. :)

This helps us validate the fix, and may help catching this or similar issues in the future.

@ferrarimarco ferrarimarco marked this pull request as ready for review December 6, 2023 18:05
@rmatte
Copy link

rmatte commented Dec 6, 2023

When I look at the commit for this it only appears to include the code for "2. Add a workflow step to catch this issue."

Yep. As I explained in #4952, I'd like to see a failing workflow, that we'll fix by committing the change you suggested. :)

This helps us validate the fix, and may help catching this or similar issues in the future.

Gotcha, thanks.

@ferrarimarco ferrarimarco marked this pull request as draft December 6, 2023 18:06
@kftsehk
Copy link
Contributor

kftsehk commented Dec 6, 2023

Had another local run issue #4954 related to the same pr #4889

So I added ci/cd tests for all non-default general super-linter behavior settings, in the hope to test local run more rigorously for blocking bugs, see PR #4955

@ferrarimarco ferrarimarco added the O: backlog 🤖 Backlog, stale ignores this label label Dec 7, 2023
@ferrarimarco ferrarimarco changed the title Fix find when linting non-git repos Don't forcefully validate Git repos if not needed Dec 7, 2023
@ferrarimarco ferrarimarco marked this pull request as ready for review December 7, 2023 13:13
Copy link
Contributor

@zkoppert zkoppert left a comment

Choose a reason for hiding this comment

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

👍🏻

@ferrarimarco ferrarimarco added this pull request to the merge queue Dec 7, 2023
Merged via the queue into main with commit 7a21f93 Dec 7, 2023
3 checks passed
@ferrarimarco ferrarimarco deleted the fix-find branch December 7, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working O: backlog 🤖 Backlog, stale ignores this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git repository validation should not be enforced when USE_FIND_ALGORITHM=true
4 participants