Skip to content

[GitHub] split enterprise-specific methods into a separate file. #4100

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rgmz
Copy link
Contributor

@rgmz rgmz commented Apr 26, 2025

Description:

The github.go file is large and unwieldy. Per #3298 (comment), Chunks is only used by the enterprise code and so I've marked it as deprecated and moved it into a separate file.

+ @mcastorina

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

@rgmz rgmz requested review from a team as code owners April 26, 2025 23:58
@rgmz rgmz force-pushed the chore/github-deprecate-chunks branch 2 times, most recently from b42e117 to 1a1febe Compare April 27, 2025 00:04
@rgmz rgmz force-pushed the chore/github-deprecate-chunks branch from 1a1febe to 5051b44 Compare May 1, 2025 14:10
Copy link
Collaborator

@rosecodym rosecodym left a comment

Choose a reason for hiding this comment

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

Unfortunately, the migration referred to by #3298 (comment) is more complex than just lighting a new code path up. Chunks is deprecated except for target scanning - full deprecation will require moving that functionality elsewhere, and we don't yet know where that will be. It might not even remain enterprise-only.

This PR splits the source into two pieces, only one of which is relevant to OSS contributors. But because the dividing line is merely a historical artifact - it doesn't represent any underlying structure - it's extremely porous, and I predict that it won't take long for something to end up on the "wrong" side of it, at which point our big ball of mud will be even less comprehensible for being split into two somewhat arbitrary files.

If you want to stop having to look at code that's not relevant to you, I think a better approach would be to actually delete it: figure out a way to get rid of scan. Quarantining it and waiting for it to die of natural causes (as you've done in this PR) isn't going to be sufficient; if we want it gone, we'll need to resort to murder.

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.

2 participants