-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Remove include repos #4469
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
base: main
Are you sure you want to change the base?
Remove include repos #4469
Conversation
@sysread @rosecodym Here is the follow up OSS pr i mentioned to remove the interactions with the removed IncludeRepos line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall 👍 I just had one question: since include-repos
is no longer used in the engine, should we also remove the corresponding CLI flag from main.go
? Right now it seems like users could still set the flag, but it wouldn’t have any effect. If there’s a reason for keeping it around (e.g., backward compatibility), then all good, just wanted to double-check.
@shahzadhaider1 I believe that the gitlab integration uses include-repos as well, and I didn't add include-repos to main.go as part of my initial change. I think, to that end, we should likely keep it around for now. Let me know if you need more info! |
Description:
Removed redundant IncludeRepos references from OSS, to match the changes requested in the Enterprise codebase. (https://github.com/trufflesecurity/thog/pull/4996) This is a continuation of #4430, where these changes were part of my original fix. Looking back, the changes here undo the unnecessary work from that PR, while leaving the load-bearing changes that fixed the original issue.
Checklist:
make test-community
)?make lint
this requires golangci-lint)?