chore: refactor resolvers #2794
Conversation
github-actions
bot
added
the
area/tooling
miparnisari
force-pushed
the
refactor-resolvers
branch
from
4b10e51 to
8eb66d5
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2794 +/- ##
==========================================
- Coverage 77.71% 77.69% -0.02%
==========================================
Files 472 472
Lines 49709 49708 -1
==========================================
- Hits 38627 38616 -11
- Misses 8231 8237 +6
- Partials 2851 2855 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -62,21 +62,21 @@ type SchemaReference struct { | |||
| TargetNamePositionOffset int | |||
| } | |||
|
|
|||
| // Resolver resolves references to schema nodes from source positions. | |||
| type Resolver struct { | |||
There was a problem hiding this comment.
i was confused by the existence of this struct and pkg/schema/resolver.go so i renamed both
| // Check the caveat, if any. | ||
| if allowedRelation.GetRequiredCaveat() != nil { | ||
| _, err := def.TypeSystem().resolver.LookupCaveat(ctx, allowedRelation.GetRequiredCaveat().CaveatName) | ||
| _, err := def.TypeSystem().GetCaveat(ctx, allowedRelation.GetRequiredCaveat().CaveatName) |
There was a problem hiding this comment.
fixed this code smell: https://refactoring.guru/smells/message-chains
There was a problem hiding this comment.
Like in the sense that it prevents the caller from needing to reach into the struct? I agree with the refactor but I don't really understand what the link is asserting.
tstirrat15
left a comment
tstirrat15
left a comment
There was a problem hiding this comment.
This makes sense to me. I think I'd want to get Barak or Joey's eyes on it.
| // Check the caveat, if any. | ||
| if allowedRelation.GetRequiredCaveat() != nil { | ||
| _, err := def.TypeSystem().resolver.LookupCaveat(ctx, allowedRelation.GetRequiredCaveat().CaveatName) | ||
| _, err := def.TypeSystem().GetCaveat(ctx, allowedRelation.GetRequiredCaveat().CaveatName) |
There was a problem hiding this comment.
Like in the sense that it prevents the caller from needing to reach into the struct? I agree with the refactor but I don't really understand what the link is asserting.
No functional changes.