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

Spring cleaning - removing dead code and other source tree care #1201

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

anderbubble
Copy link
Collaborator

@anderbubble anderbubble commented Apr 20, 2024

Description of the Pull Request (PR):

Holistic update and cleanup for the source tree. Mostly removing dead code using the deadcode utility from https://go.dev/blog/deadcode.

deadcode -test ./...

Also updated LICENSE_DEPENDENCIES.md and moved a few minor files around.

Before submitting a PR, make sure you have done the following:

@anderbubble
Copy link
Collaborator Author

@mslacken this is something I've been thinking about for a while, but I hadn't found a good tool to do it with. Tonight I found deadcode, which seems to do what I've been looking for.

There's a fair bit removed here, so I understand if it sparks some concern. Some of the removed code is reasonably expected, just unused; for now, I took the tact that if it's not tested then it's more risk than benefit. But if code was in the testsuite then I considered it used, even if it's not used anywhere else.

I see there's some linter failures, so I'll address those.

Let me know what you think.

https://go.dev/blog/deadcode

```
deadcode -test ./...
```

Confirmed all tests still pass and the build still succeeds.

Signed-off-by: Jonathon Anderson <janderson@ciq.com>
Signed-off-by: Jonathon Anderson <janderson@ciq.com>
@anderbubble anderbubble changed the title Purge Spring cleaning - removing dead code and other source tree care Apr 20, 2024
@mslacken
Copy link
Member

Prince Arthas might not be happy with this.
Besides from that, having less coffee is always better and a great idea.
Can we postpone this after we have:

  • 4.5.1
  • Merged in the dracut code
  • Removed NodeInfo
    Especially having removed NodeInfo will create a lot more of dead code.

I also would like to ask you if there is a tool with which we can reduce vendor depencies?

@anderbubble
Copy link
Collaborator Author

Definitely fine for this to wait until after v4.5.1.

For NodeInfo, I'd say let's go ahead and merge this in, and then just do it again after the NodeInfo refactor.

I got the errors cleared.

@anderbubble
Copy link
Collaborator Author

For NodeInfo, I'd say let's go ahead and merge this in, and then just do it again after the NodeInfo refactor.

Eh, I'm probably wrong. This can wait until after NodeInfo. This was really easy to do, and I could see it being really disruptive to the NodeInfo refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants