-
Notifications
You must be signed in to change notification settings - Fork 0
Improve error checks when we start a watch #68
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: main
Are you sure you want to change the base?
Conversation
9202f07
to
e7e2d3f
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #68 +/- ##
=========================================
+ Coverage 79.6% 80.0% +0.3%
- Complexity 174 176 +2
=========================================
Files 23 23
Lines 766 770 +4
Branches 88 89 +1
=========================================
+ Hits 610 616 +6
+ Misses 95 92 -3
- Partials 61 62 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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!
throw new IllegalStateException("There is no onEvent handler defined"); | ||
} | ||
if (!Files.exists(path)) { | ||
throw new FileSystemException(path.toString(), null, "Cannot open a watch on a non-existing path"); |
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.
Nit: NoSuchFileException
is more accurate? (also in two places below)
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.
true, but that doesn't allow us to pas in which path it more accurate. so thats why I went for their parent.
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.
I think I might be misunderstanding your point, so at the risk of telling you something you already know 😉: NoSuchFileException
has a constructor with the same arguments as FileSystemException
.
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.
Ah right, I first added the one for the directory exception and NotDirectoryException
did not have this overload, so I went to it's parent and stayed there.
Co-authored-by: sungshik <[email protected]>
No description provided.