Closed krze closed 6 years ago
loudmouth
commented
6 years ago Hey @krze thanks so much for doing all this work! I'll review the changes today!
As you may have gathered by looking at the other Github issues, this repo is no longer receiving full support from Contentful, but if the maintenance burden is low maybe we can figure out a way to keep the repo building and working.
Out of curiosity, are you using Concorde for a personal project or for more professional work?
krze
commented
6 years ago In this case it was professional, I was investigating progressive JPG solutions for our existing image stack and Concorde looked like a good option. It's not in production but we were pleased about how easily it integrated. We were actually kind of shocked how readily it worked because our stack is a bit ancient.
Still really cool stuff, libjpeg is interesting to work with
loudmouth
commented
6 years ago cool! It may be that I get to this review tomorrow rather than today as other priorities are more pressing.
The reason I asked about your use case is that I'm trying to figure out the future of this project. You may have noticed that's under our -labs GitHub org which means it is officially an "experimental unofficial" project by our organization's standards and needs. Depending on your interest level, maybe we can get in contact outside of GitHub and ponder the future of Concorde together...
loudmouth
commented
6 years ago Hey @krze, the PR looks great, but I don't want to merge until the tests are passing again. Unfortunately, I don't have time at the moment to work on passing the tests, but if you are willing ot keep going and get everything passing again, I'll be happy to merge this PR.
Once again, thanks so much for all the hard work 🙏
krze
commented
6 years ago @loudmouth Added the nullability checks and got the tests to run. The Snapshot tests are failing in CI but passing locally, despite using the same simulator in both environments. I'm not sure why they're being picky in CI - running make setup then make test will result in 100% pass locally
krze
commented
6 years ago Scratch that: added a env var check in the tests to generate snapshots in CI. The tests will now clean and generate snapshots before running the test for real. Exit status is determined from the second test run
loudmouth
commented
6 years ago Wow. Thanks so much @krze for taking this across the finish line!
I did a little grunt work here converting the project to Swift 4 from Swift 2. There's still a warning regarding the deprecation of NSURLConnection. The CCBufferedImageView needs refactoring to be up to date with URLSession, but that's going to be a bigger effort.
I figured the first step would be to make this repo easily compatible with Xcode 9 😛
The tests all pass, I regenerated the reference images for the snapshot cases and they match the spec.