search

JamitLabs / Accio

A dependency manager driven by SwiftPM that works for iOS/tvOS/watchOS/macOS projects.
MIT License
664 stars 32 forks source link

Re-download checkout if failing to reset #40

Closed fredpi closed 5 years ago

fredpi commented 5 years ago

Fixes #27 using a pragmatic approach.

As Accio currently fails to build (maybe only on my machine?) due to a build error in xcodeproj, this is not tested yet.

fredpi commented 5 years ago

Given that we have git -C calls elsewhere too (and it's always the same three calls), shouln't we refactor this to a method and do the try-catch for all cases? The method might be throwing so we can handle the error case on the usage side.

If I looked it up properly, the other git -C calls happen after Swift PM resolves the dependencies. So if we delete the checked out repo, Swift PM won't come to the rescue and redownload it just afterwards. This is why I think, it's only suitable to apply this handling to the git -C commands run before DependencyResolverService.shared.resolveDependencies() is performed.

Jeehut commented 5 years ago

@fredpi As I've already stated:

The method might be throwing so we can handle the error case on the usage side.

What I mean there is that the refactored method should actually not include a do-catch but instead a try and be throwing. Then whenever we know what to do as an alternative when the reset fails, we could act accordingly (like deleting checkouts in the one case).

fredpi commented 5 years ago

@Dschee Sorry, I didn't read carefully -.-

Jeehut commented 5 years ago

No problem, I may have been ambiguous or at least not so clear. 😅

fredpi commented 5 years ago

Should be good to merge now

Jeehut commented 5 years ago

Please also remove the [WIP] in the title when it's ready for merge. :)

Jeehut commented 5 years ago

Looks good now, merging. Thanks for the PR! 🎉

Jeehut commented 5 years ago

Oh no, we forgot the Changelog entry. 😄