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

Removing Manager and improve NewConfiguration typesafety #215

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

abjeni
Copy link
Collaborator

@abjeni abjeni commented Mar 12, 2025

This is a prototype of the different design that uses a configuration in place of a manager.
The raw configuration is now a struct that points to a raw manager as well as the slice of raw nodes.
The NewConfiguration method takes a set of manager options instead of requiring a manager.
RawManager should be made private since it is no longer used by the generated files.
Sub configurations are created with a SubConfiguration method, without manager options.
There is no main configuration, the sub configurations all belong to the same raw manager.
The Close method is used to close a configuration along with all of its sub configurations.

Notes

  • NewConfiguration should only accept node map and node list.
  • mgr_test.go and config_test.go uses RawManager directly outside of the gorums package.
  • the storage example benefits from having a manager.
  • tests/config/config_test.go need a way to create an empty configuration.

Fixes #192

Copy link
Contributor

deepsource-io bot commented Mar 12, 2025

Here's the code health analysis summary for commits ca1cee9..125b0cf. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Go LogoGo❌ Failure
❗ 19 occurences introduced
View Check ↗
DeepSource Shell LogoShell✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

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.

The generated Manager.NewConfiguration method is fragile
1 participant