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

GOLD-277 : Required minimum for consensus should be STATIC not dynamic number #308

Closed
wants to merge 1 commit into from

Conversation

@abdulazeem-tk4vr abdulazeem-tk4vr self-assigned this Oct 2, 2024
Copy link

github-actions bot commented Oct 2, 2024

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Code Clarity
The added TODO comments suggest future improvements but lack immediate implementation. This could lead to confusion or forgotten tasks. It's recommended to either implement the changes now or track them more formally via a task management system.

Consistency Check
The new checks for minimum consensus group size are repeated across multiple methods. Consider refactoring this into a single method to improve maintainability and reduce code duplication.

Error Handling
The new code adds checks for the execution group size but does not handle the case when the size is below the minimum threshold. It's crucial to implement error handling or fallback logic to manage these scenarios effectively.

@abdulazeem-tk4vr abdulazeem-tk4vr force-pushed the GOLD-277 branch 2 times, most recently from 69b1ba1 to acbf139 Compare November 6, 2024 06:37
@aniketdivekar
Copy link
Contributor

closing in favour of this PR

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.

2 participants