-
Notifications
You must be signed in to change notification settings - Fork 53
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
Addressing missed feedback from #551 and returning FilePath #553
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #553 +/- ##
=======================================
Coverage 67.99% 68.00%
=======================================
Files 139 139
Lines 23722 23729 +7
=======================================
+ Hits 16130 16137 +7
Misses 4537 4537
Partials 3055 3055
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm modulo a few minor fixes
internal/compare/compare.go
Outdated
file, err := filepath.Rel(p.GitDir, filePath) | ||
if err != nil { | ||
// If a file was deleted, then we will not be able to | ||
// find a full path to it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you mean to say we will not be able to find a relative path to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup yup. thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM minus minor sung's comments and the missing tt := tt
in a test that uses t.Parallel()
.
Co-authored-by: Sung Yoon Whang <[email protected]>
Addressing leftover comments and changing thriftbreak to return filepath relative to the passed in repository.