Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Git Repos #288

Merged
merged 1 commit into from
Apr 8, 2023
Merged

Conversation

techman83
Copy link
Member

Many assumptions around branches and repos were made early on, since then we moved a lot of the common repo actions into relevant classes. This furthers that work and replaces all direct branch access with methods that automatically figure out the primary branch of the repo. Tests updated to reference 'main'

It is intended that #283 is reviewed and merged first, then once happy we can move onto this in preparation for adding KSP2 config. We will be able to follow soon after by renaming master -> main for the existing KSP repos.

@techman83 techman83 force-pushed the refactor/git-repos branch 2 times, most recently from ee03890 to 304f601 Compare March 27, 2023 13:47
@techman83 techman83 marked this pull request as ready for review April 7, 2023 04:03
@techman83 techman83 requested a review from HebaruSan April 7, 2023 04:42
Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment about resurrected code in SpaceDockAdder aside, this looks solid and ready to merge. 🎉

Many assumptions around branches and repos were made early on, since then we moved
a lot of the common repo actions into relevant classes. This furthers that work and
replaces all direct branch access with methods that automatically figure out the
primary branch of the repo. Tests updated to reference 'main'
Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, let's go with this. 🏍️

@techman83
Copy link
Member Author

Resolved the bad merge conflict resolution, and removed all the reformatting. I think I'd like to switch to black for the project, but I'd rather not mix in formatting with refactoring.

@techman83 techman83 merged commit 0856ac9 into KSP-CKAN:master Apr 8, 2023
@techman83 techman83 deleted the refactor/git-repos branch April 8, 2023 04:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants