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

NamingConsensus #1759

Merged
merged 36 commits into from
Jan 5, 2024
Merged

NamingConsensus #1759

merged 36 commits into from
Jan 5, 2024

Conversation

nimmolo
Copy link
Contributor

@nimmolo nimmolo commented Dec 31, 2023

New PORO addressing #1722

This PR moves and centralizes all code that handles naming and voting on observations, or determining the current state of naming/voting for the front end.

It loads an observation's namings and votes into a new NamingConsensus PORO in order to isolate the calculations involved in determining the observation's "consensus" naming and safeguard against any unintentional db loads whenever the namings or votes change or are calculated. Current form updates do something like 3*N+1 db loads per update, which causes a very noticeable slowdown — something like 10s on my machine/connection. Even displaying a show_obs page executes many duplicate expensive db loads.

This covers pretty central functionality to MO, and although I don't expect anyone will review it, I hope everyone will.

Many thanks to Jason for inspiration and guidance through the whole process.


Notes:

Deletes NamingParams class — moves all methods either back into NamingsController, the only place they're used, or the NamingConsensus PORO to reduce indirection and duplication.

Moves all voting methods out of the Naming and Observation classes, into NamingConsensus.

@coveralls
Copy link
Collaborator

coveralls commented Jan 1, 2024

Coverage Status

coverage: 94.586% (-0.004%) from 94.59%
when pulling 80c5574 on nimmo-election-naming
into 95a62cc on main.

@nimmolo nimmolo marked this pull request as ready for review January 5, 2024 00:50
@nimmolo nimmolo merged commit 8015bb9 into main Jan 5, 2024
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants