-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
orchard.classpath
: don't fail on directories ending on .jar
#116
Conversation
@@ -22,22 +22,29 @@ | |||
(deftest classpath-test | |||
(testing "Classpath" | |||
(testing "URLs are absolute file paths" | |||
(is (every? #(.isAbsolute (File. (.getPath %))) |
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.
Changed all instances of is every?
to is
inside a doseq
.
This way:
- the precise thing that failed is reported
- we avoid huge failure reports, especially now that we added the
"unzipped-jdk-source"
resource 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.
Agreed!
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.
Btw, I'd even suggest adding this as a guideline to the Clojure Style Guide
under "Testing".
src/orchard/misc.clj
Outdated
(some (fn [ext] | ||
(.endsWith (.. file getName toLowerCase) ext)) | ||
exts))) | ||
(and (some (fn [ext] |
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.
Doesn't it make more sense to swap the order of changes - check first that something is not a folder? That reads better to me, and probably won't have much impact on performance.
This needs a changelog entry and we're good to go. |
I pressed "Close" instead of "Comment". Sorry! |
:) yeah it's something that pops up often in code review. Might contribute at some point. Feedback applied as a git |
Thanks! |
🍻! Would appreciate a release if possible - this was a nuisance while hacking on refactor-nrepl (I had that .jar dir auto-generated for some unrelated reason, causing an error in refactor-nrepl code) |
Sure, I'll cut 0.7.1 as soon as I can. |
The new release is out. |
I recently hit the corner case of a directory named
foo.jar
which caused orchard to choke on it.This PR proposes a fix.