Fix cache directory resolution when using remote config file #3231
Conversation
In the previous version, the absolute path resolution was called on a fragment of the path, resulting in SwiftLint creating the cache in a different path from the one used to search for the cache.
This should help prevent future issues building the path.
Here's an example of your CHANGELOG entry: * Fix cache directory resolution when using remote config file.
[mokagio](https://github.com/mokagio)
[#issue_number](https://github.com/realm/SwiftLint/issues/issue_number)note: There are two invisible spaces after the entry's text. Generated by 🚫 Danger |
|
@mokagio Thanks for trying out the PR and fixing this! 🚀 Apart from the little restructuring you did, the only change seems to be the removal of Thanks again for fixing it – looks good to me and I'm going to merge it into my branch. If you want, you can create another PR, adding your name to the changelog section in my branch. 👍 |
|
Thank you for the quick review 🙌
I'd say that's likely. I experienced different error messages depending on whether the |
Now that my PR has been merged into it realm/SwiftLint#3231
Now that my PR has been merged into it realm/SwiftLint#3231
Now that my PR has been merged into it realm/SwiftLint#3231
I'm spiking using a centralized SwiftLint configuration across the Automattic's Swift repositories, as we have quite a few.
I used #3058 but it wasn't working. After a bit of digging around, I discovered the issue had to do with how the cache directory path was build, or to be precise resolved as an absolute path.
You can see the SwiftLint build failing here (although the message is a bit misleading) and succeeding here, with the only change being that the former uses #3058 as the source for SwiftLint and the latter this PR.
I would have liked to add tests for this fix, but the only way I could see would have been to add an abstraction on
FileManagerin order to provide a test double for it in the test. That seemed like a lot of work as well as a substantial change in the design of this code, which is not onmasteryet. I opted for simply fixing the production code, instead. If you can think of a quick way to add tests which would fit in the context of this PR, I'd love to make that happen.Thank you for working on this @fredpi. It's super useful, I really love this feature.