-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[WIP] Refactor Tourney classes #16090
base: master
Are you sure you want to change the base?
Conversation
isaacl
commented
Sep 20, 2024
•
edited
Loading
edited
- Move recently created complex logic into its own object
- Add a deterministic tourney stagger method
- Add stagger to Plan objects, rather than Tournaments to be more easily testable.
90f100d
to
37bc919
Compare
d49e666
to
aa748a8
Compare
This probably works, though I haven't tested. However it's kind of a hot mess of code. I'm struggling to figure out how to simplify. I don't really need a TreeSet for the tiny list of tourneys, but even without that structure, the code looks the same. This whole change is probably overkill, but the idea was:
Still, it feels like a lot of verbose code for a relatively small issue :-\ so not sure what to do. |
aa748a8
to
174570e
Compare
- Move recently created complex logic into its own object - Add a deterministic tourney stagger method - Add stagger to Plan objects, rather than Tournaments to be more easily testable.
174570e
to
d9254df
Compare
It's a lot of tricky code to review. I did read it quickly and found nothing alarming. I deleted my local tournaments and let it re-create them. They look fine, and the deterministic stagger is certainly nice. Surely there are some bugs lurking, and it's ok to find them later, in prod. |
I’ll be adding tests of the new code before it’s ready to land. Will follow up. |