Closed tom95 closed 4 years ago
LGTM.
Please make sure that the preferences are ok after a bootstrap including this PR. There should be none dangling on the unused MCGitRepo... (or so, I think) :)
@krono This is the result of a clean install on 5.3: the old preferences are still there because people can still use the old repo types should they prefer that. I would do a separate PR maybe after the next stable Squeak release that removes the old repo types so that we can be more confident that the new code is working well and there should be no need to opt for the old version.
Otherwise it's looking good after bootstrapping.
During the migration from the filetree-only git repos to the more general ones we did not copy the necessary accessors for the preferences to be saved and read correctly. Further, we used ClassVars instead of class-side InstVars, which meant that BitBucket and GitHub always shared the same folder. This is now repaired.
I dare claim that a migration script is not required here since setting the preference had been broken, so there's good reason to assume that people left it at the default thus far.
Note that the preferences for the cache paths will now be duplicated (i.e. you will have one "GitHub Cache" and one "Fetch GitHub Cache"). I gave them a description to inform users which one to use, which I am aware is not ideal but I think should work well enough. The next step would be to assess if the former GitHub and BitBucket classes still have value for a default installation in Squeak or whether our refactoring supersedes them in all use cases. If so, we can make the baseline not load these for Squeak, thus removing the duplicate preferences.