Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Compiled the resources #51

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Jerakin
Copy link
Collaborator

@Jerakin Jerakin commented Feb 21, 2020

Pros

  • No need to change the path to the resources for cx_Freeze and PyInstaller
  • User will not have to include qtmodern resources when bundling

Cons

  • Slower to do iterations on the resources because they now require an additional compile step

All in all makes it a lot "it just works" for users, but requires a bit of extra work from the maintainers.

Copy link
Owner

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

Compiled resources depend on PyQt5, what if I use PySide2?

@Jerakin
Copy link
Collaborator Author

Jerakin commented Feb 21, 2020

Well spotted! Should depend on qtpy

@Jerakin Jerakin force-pushed the compiling_resources branch from 1611605 to b85a246 Compare February 21, 2020 10:48
@razaqq
Copy link
Contributor

razaqq commented Feb 21, 2020

also makes it a bit harder for the user to make changes to stylesheet etc, not that that matters for most people, but maybe include the uncompiled resources in the next PyPI release too?

@Jerakin
Copy link
Collaborator Author

Jerakin commented Feb 21, 2020

That's a good point, we should maybe do something like this.

https://github.com/Jerakin/qtmodern/compare/compiling_resources...Jerakin:change_style?expand=1

That way they could at least monkey patch in their own stylesheet

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants