Fix tests #5
Fix tests #5
Conversation
Sources/XCHammer/Generator.swift
Outdated
| if parent.pathExtension != ext && url.absoluteString.contains(ext) { | ||
| (ext, dotExt) in | ||
| if parent.pathExtension != ext && | ||
| url.absoluteString.contains(dotExt) { |
There was a problem hiding this comment.
I think it would be cleaner to check if there is a path component that ends with the extension
There was a problem hiding this comment.
I agree and that had a major performance hit
There was a problem hiding this comment.
Can we just add it as an additional && to this statement? That way you'll only walk the path components of a path that includes the extension
There was a problem hiding this comment.
What do you think the suggested code would do when we have a path containing pinterest/ios/app?
The code checks for material within some.app, not for paths that include app.
Also, as I mentioned, the effect of creating pathComponents was a major performance hit.
Sources/XCHammer/Generator.swift
Outdated
| @@ -369,11 +369,13 @@ enum Generator { | |||
| // Skip hashing children of Xcode directory like files | |||
| private static let xcodeDirExtensions = ["app", "appex", "bundle", "framework", "octest", | |||
| "xcassets", "xcodeproj", "xcdatamodel", "xcdatamodeld", | |||
| "xcmappingmodel", "xctest", "xcstickers", "xpc", "scnassets" ] | |||
| "xcmappingmodel", "xctest", "xcstickers", "xpc", "scnassets" ].map { ($0, "." + $0 ) } | |||
There was a problem hiding this comment.
I don't think there is there a lot of value into having a tuple to add a "." prefix
There was a problem hiding this comment.
Checkout how the previous line of code is consuming this - it is needed.
There was a problem hiding this comment.
And by previous, I mean 376 😄
There was a problem hiding this comment.
I see how it's used but it just seems a bit strange for a list ofxcodeDirExtensions to have two representations of it. If you really want that then you should probably just create struct type wrapper that has a convenience method for the extension prefixed with .
jerrymarino
force-pushed
the
jmarino_fix_tests
branch
from
764da90 to
f06d300
Compare
We have missed a few things which broke the tests and Makefile
Make sure tests are passing, and fix a hash issue which impacted
ios-app.