-
Notifications
You must be signed in to change notification settings - Fork 19
Fix annotations in file note functions #1220
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
base: master
Are you sure you want to change the base?
Conversation
065dd97
to
c2b17d0
Compare
if isAbsolute filePath | ||
then H.annotate filePath | ||
else H.annotate $ cardanoCliPath </> filePath |
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.
The check is redundant when using </>
if isAbsolute filePath | |
then H.annotate filePath | |
else H.annotate $ cardanoCliPath </> filePath | |
H.annotate $ cardanoCliPath </> filePath |
if isAbsolute filePath | ||
then H.note filePath | ||
else do | ||
let tempWithFilePath = tempDir </> filePath | ||
if isAbsolute tempWithFilePath | ||
then H.note tempWithFilePath | ||
else do | ||
H.annotate $ cardanoCliPath </> tempWithFilePath | ||
return tempWithFilePath |
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.
Why are we annotating absolute paths, but returning a relative one only in this place? What issue does this function solve?
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.
isAbsolute
seems unnecessary
Changelog
Context
The annotations were incorrect in the case of absolute file paths. Absolute file paths should not be prepended to.
How to trust this PR
Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.
Checklist