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

Controversial change... #37

Merged
merged 1 commit into from
Oct 26, 2021
Merged

Conversation

mellowdrifter
Copy link
Contributor

There may be pushback for this change, but hear me out. Two reasons I moved main to run:
1 - This is a good overview as to the why: https://pace.dev/blog/2020/02/12/why-you-shouldnt-use-func-main-in-golang-by-mat-ryer.html
2 - main() is currently 218 lines long. As as noted before it's difficult to test. Doing it this way makes it easier.

Long term, the vast majority of the initial set up should be in their own functions and run() should merely call these functions to set up that state, then downloading VRPs, then sitting and waiting for clients.

@job
Copy link
Member

job commented Oct 26, 2021

@ties @randomthingsandstuff this is up to you

@randomthingsandstuff
Copy link
Contributor

Yeah this isn't controversial from my POV. I mean, its low-hanging stuff to make testing a little bit better. Approve from my side.

@randomthingsandstuff randomthingsandstuff merged commit 683f7f2 into bgp:master Oct 26, 2021
@ties
Copy link
Collaborator

ties commented Oct 26, 2021

Not controversial to me either. The other refactoring (args) proposed in that blog also looks good to me.

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.

4 participants