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

Unify build.py contents #1524

Merged
merged 1 commit into from Dec 14, 2018
Merged

Unify build.py contents #1524

merged 1 commit into from Dec 14, 2018

Conversation

ghost
Copy link

@ghost ghost commented Dec 12, 2018

This unifies the different build.py files to get the different options in sync, and make it easier to expand them without all the duplicate code.

Tested it, and it works fine with my app / sdl2 bootstrap!

@ghost ghost changed the title [WIP] Unify build.py contents Unify build.py contents Dec 13, 2018
@ghost ghost changed the title Unify build.py contents [WIP - investigating potential issue] Unify build.py contents Dec 13, 2018
@ghost ghost changed the title [WIP - investigating potential issue] Unify build.py contents Unify build.py contents Dec 13, 2018
Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

I feel like that actually makes it hard to extend with new bootstraps or update existing one.
Would it not be better approach to have some common logic somewhere and slight specific overriding changes in specific bootstraps?
Like using specialisation inheritance, would it make sense?

@ghost
Copy link
Author

ghost commented Dec 13, 2018

That definitely would make sense. (I like the idea of inheritance! Have been thinking about it as well) However, similarly to the common bootstrap area merge request which could have incorporated this but didn't, I personally prefer to go about this in multiple separate requests. While I agree it looks a bit messy with the conditionals, it is IMHO still a lot easier to do changes now, given it's at least in one central obvious place of what the bootstrap differences are

@AndreMiras AndreMiras self-assigned this Dec 14, 2018
Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

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

Looking good, thanks for another great contribution 🍻

@AndreMiras AndreMiras merged commit 9b9bcf8 into kivy:master Dec 14, 2018
@ghost ghost deleted the unify-build-py branch December 14, 2018 21:46
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.

1 participant