Skip to content
This repository was archived by the owner on Apr 23, 2026. It is now read-only.

ci: Don't require security-lint for auto-merge#3353

Closed
booxter wants to merge 1 commit intoinstructlab:mainfrom
booxter:dont-require-security-lint
Closed

ci: Don't require security-lint for auto-merge#3353
booxter wants to merge 1 commit intoinstructlab:mainfrom
booxter:dont-require-security-lint

Conversation

@booxter
Copy link
Copy Markdown
Contributor

@booxter booxter commented May 2, 2025

The check has known false positives 1, so we cannot make it a
requirement for mergify. It's an informational signal for reviewers that
additional scrutiny may be justified.

Signed-off-by: Ihar Hrachyshka [email protected]

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the
    conventional commits.
  • Changelog updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Functional tests have been added, if necessary.
  • E2E Workflow tests have been added, if necessary.

The check has known false positives [1], so we cannot make it a
requirement for mergify. It's an informational signal for reviewers that
additional scrutiny may be justified.

[1]: instructlab/ci-actions#14 (comment)

Signed-off-by: Ihar Hrachyshka <[email protected]>
@booxter booxter marked this pull request as draft May 2, 2025 21:08
@mergify mergify Bot added the CI/CD Affects CI/CD configuration label May 2, 2025
@booxter booxter marked this pull request as ready for review May 6, 2025 16:26
@booxter
Copy link
Copy Markdown
Contributor Author

booxter commented May 6, 2025

Unblocking this to allow merging; I believe this is the right thing to do, knowing that the check is designed to trigger false positives, and so it cannot be used as a merge requirement and/or block running other jobs.

@booxter booxter marked this pull request as draft May 6, 2025 22:21
@booxter
Copy link
Copy Markdown
Contributor Author

booxter commented May 6, 2025

Maybe I won't need after all if I get away without GH_TOKEN (that may not be needed for just checking out the current PR from git.)

@ktdreyer
Copy link
Copy Markdown
Contributor

ktdreyer commented May 8, 2025

I am ok with this, based on my analysis in instructlab/ci-actions#14

Agreed it would be great to remove our use of GH_TOKEN entirely.

@mergify mergify Bot added the one-approval PR has one approval from a maintainer label May 8, 2025
@booxter booxter requested a review from courtneypacheco May 14, 2025 19:33
@booxter
Copy link
Copy Markdown
Contributor Author

booxter commented May 31, 2025

I don't have capacity to complete it.

@booxter booxter closed this May 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CI/CD Affects CI/CD configuration one-approval PR has one approval from a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants