Skip to content

Using comma-ok pattern for rule evaluation - #10

Merged
serroba merged 1 commit into
mainfrom
dev
Jan 2, 2026
Merged

Using comma-ok pattern for rule evaluation#10
serroba merged 1 commit into
mainfrom
dev

Conversation

@serroba

@serroba serroba commented Jan 2, 2026

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI review requested due to automatic review settings January 2, 2026 06:51
@codecov

codecov Bot commented Jan 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (747a94f) to head (17ae615).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main       #10      +/-   ##
===========================================
+ Coverage   92.22%   100.00%   +7.77%     
===========================================
  Files           6         6              
  Lines         193       176      -17     
===========================================
- Hits          178       176       -2     
+ Misses         15         0      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@serroba
serroba merged commit 4d67970 into main Jan 2, 2026
8 checks passed
@serroba
serroba deleted the dev branch January 2, 2026 06:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the rule evaluation logic to use Go's idiomatic comma-ok pattern instead of returning pointer values. The main changes update the RuleMatcher function signature from returning *Rule to (Rule, bool), making the code more idiomatic and avoiding nil pointer checks. Additionally, route registration has been extracted into a separate file.

Key Changes

  • Changed RuleMatcher function signature to return (Rule, bool) instead of *Rule
  • Updated DefaultRuleMatcher implementation to use range iteration and return value semantics
  • Extracted HTTP route registration from handler.go into a new routes.go file

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/flags/evaluator.go Updated RuleMatcher type signature and DefaultRuleMatcher implementation to use comma-ok pattern and value semantics
internal/flags/service.go Updated Evaluate method to use comma-ok pattern when calling rule matcher; refactored NewService to delegate to NewServiceWithMatcher
internal/flags/evaluator_test.go Updated all test cases to use comma-ok pattern and verify boolean return values instead of checking for nil
internal/flags/service_test.go Updated custom matcher function to match new signature with (Rule, bool) return
internal/handler/handler.go Removed Register method and net/http import (moved to routes.go)
internal/handler/routes.go New file containing extracted Register method for HTTP route registration
codecov.yml Added routes.go to coverage ignore list
.testcoverage.yml Added routes.go to coverage exclude paths

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .testcoverage.yml
# Main entry points and DI wiring
paths:
- ^cmd/server/main\.go$
- ^internal/handler/routes\.go

Copilot AI Jan 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exclude pattern is missing the dollar sign anchor at the end. This pattern should use the same format as the existing pattern for consistency and to avoid accidentally matching other files that start with this path. The pattern should be ^internal/handler/routes\.go$ to match only this specific file.

Suggested change
- ^internal/handler/routes\.go
- ^internal/handler/routes\.go$

Copilot uses AI. Check for mistakes.
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.

2 participants