feat: add arrow reversal optimization #2821
Conversation
github-actions
bot
added
the
area/tooling
barakmich
force-pushed
the
barakmich/arrow_reversal
branch
from
5d267b0 to
10e3a78
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2821 +/- ##
==========================================
- Coverage 75.80% 75.78% -0.01%
==========================================
Files 459 460 +1
Lines 54848 54922 +74
==========================================
+ Hits 41570 41617 +47
- Misses 10400 10417 +17
- Partials 2878 2888 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| intermediateAsSubject := ObjectAndRelation{ | ||
| ObjectType: rightPath.Resource.ObjectType, | ||
| ObjectID: rightPath.Resource.ObjectID, | ||
| Relation: "", |
There was a problem hiding this comment.
What does an empty relation here mean? Is it different from an ellipsis?
| Expiration: rightPath.Expiration, | ||
| Integrity: rightPath.Integrity, |
There was a problem hiding this comment.
Still curious on this point - are we arbitrarily choosing the right for expiration and integrity, or is there a reason that it makes sense semantically?
| if rightToLeftCost < leftToRightCost && a.direction != rightToLeft { | ||
| // Create new arrow with inverted direction | ||
| newArrow := &Arrow{ | ||
| id: a.id, // Keep same ID | ||
| left: a.left, | ||
| right: a.right, | ||
| direction: rightToLeft, | ||
| } | ||
| return newArrow, true, nil // Changed | ||
| } |
There was a problem hiding this comment.
Is there a case where we'd need to switch it the other way, or is that implicit because LTR is the default?
There was a problem hiding this comment.
implicit default, yeah
barakmich
force-pushed
the
barakmich/arrow_reversal
branch
from
10e3a78 to
bea7b14
Compare
tstirrat15
left a comment
tstirrat15
left a comment
There was a problem hiding this comment.
I like the combination
Description
Testing
References