Skip to content

Fix various issues with graphs and recipes #1669

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

Merged
merged 1 commit into from Feb 5, 2019
Merged

Fix various issues with graphs and recipes #1669

merged 1 commit into from Feb 5, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 3, 2019

  • fix graph depending on capitalization of dependency/recipe names when pip doesn't
  • fix graph giving no useful errors for conflicts (now it gives them sometimes)
  • fix graph entering infinite loop when no bootstrap is found
  • fix recipe not being obtainable irregardless of capitalization
  • fix recipe IOError-on-doesn't-exist not distinguishable from
    recipe-exists-but-code-messes-up-and-causes-unintentional-IOError.
    a non-existent recipe will now cause a ValueError
  • fix the bootstraps' own dependencies being wrong and unused
  • change preferred default from python2 + sdl2 to python3 + sdl2
  • possibly other minor fixes I forgot about

(some of these fixes are required for #1625 / project's setup.py to be reasonably used)

@ghost ghost changed the title Fixes various issues with graphs and recipes Fix various issues with graphs and recipes Feb 3, 2019
@AndreMiras
Copy link
Member

Thanks for making a dedicated one for it. We'll review it more seriously later on.
Did you see that the tox test broke?

@ghost
Copy link
Author

ghost commented Feb 3, 2019

@AndreMiras thanks I had to fix some of the unit tests, it should work now!

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.

Thanks for the PR @Jonast it looks once again very good overall! 💪
I left some minor comments and less minor concerns, please let me know what you think.
I think we're almost there, just I prefer to be sure the code looks great since we're touching sensitive components

AndreMiras
AndreMiras previously approved these changes Feb 5, 2019
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'm ready for the merge, thanks for your time and effort 💪
Will merge when build turns green

- fix graph depending on capitalization when pip doesn't
- fix graph giving no useful errors (now it gives them sometimes)
- fix graph entering infinite loop when no bootstrap is found
- fix recipe not being obtainable irregardless of capitalization
- fix recipe IOError-on-doesn't-exist not distinguishable from
  code-messes-up-and-causes-true-IOError. a non-existent recipe will
  now cause a ValueError
- fix the bootstraps own dependencies being wrong and unused
- possibly other minor fixes I forgot about
Copy link
Member

@inclement inclement left a comment

Choose a reason for hiding this comment

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

Very nice, thanks!

@inclement inclement merged commit 50afdf1 into kivy:master Feb 5, 2019
@ghost ghost deleted the graphchanges branch February 8, 2019 03:58
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.

4 participants