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

Refactor code in smaller files and remove most IDE warnings #7

Closed
wants to merge 1 commit into from

Conversation

pohlm01
Copy link
Contributor

@pohlm01 pohlm01 commented Sep 18, 2024

I hope this doesn't take you too much by surprise. Let me know if you don't agree with (parts of) it.
I made an attempt to bring a bit more structure into the code and especially split up the very long mtc.go file into multiple. For now, I stored most stuff in the /pkg directory, but I would leave it up to you, if you prefer it in the /internal folder (for now).

The changes should not contain any functional changes.

@bwesterb
Copy link
Owner

I strongly prefer to keep code together in one file that's closely related, and instead split on what could've been a separate package. It makes it easier to jump back and forth between related definitions. Also, it reduces time spent thinking about what file to put new code in. At some point this becomes unwieldy, but that'd be if the file grows above say 3000 lines of code or so.

@pohlm01
Copy link
Contributor Author

pohlm01 commented Sep 18, 2024

Fine, no problem. I'll close this PR then.

@pohlm01 pohlm01 closed this Sep 18, 2024
@bwesterb
Copy link
Owner

bwesterb commented Sep 18, 2024

Btw, your IDE uses golangci-lint?

@pohlm01
Copy link
Contributor Author

pohlm01 commented Sep 18, 2024

As far as I know, it does not. I'm using the default settings of GoLand from JetBrains.

@bwesterb
Copy link
Owner

I fixed some issues raised by golangci-lint in 76c8782. I'll enable it in CI once all are fixed.

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