-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
relative paths for icon and version in spec file #3333 #3444
Conversation
Thanks for the pull-request. All test-runs are failing. I'm afraid, there is a small but serious error in the code, see https://travis-ci.org/pyinstaller/pyinstaller/jobs/362774665#L3061 |
I am mystified. I didn't change anything in the PYZ section, only EXE. And it was working fine in my tests. Well, wait, one change I did make that could possibly be related is, each class had its own copy of commit: https://github.com/tallforasmurf/pyinstaller/commit/a86349e2fdb523bc6d416fdb3bd0df1a7bfeadb3 |
PyInstaller.config.CONF gets monkeypatched in the tests. |
Wait, don't test yet. I just remembered why I made that change, it wasn't merely for tidyness. Must check. EDIT: yes, because the existing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume, this is ready for review now :-) I have some minor comments.
May I also ask you to
- squash the fix-ups (e.g. a86349e, 9e29fb0) into the commit they are fixing
- rework the commit messages and remove the relative paths for icon and version in spec file #3333 from the headline (having them in the body is okay, but 9d8c31d is not related IMHO)
Thanks.
@tallforasmurf May I ask you to clean up the pull-request and fix the open coding issues. I would like to merge this soon. Thanks. Please reword your commit messages to comply to our Commit Message Rules - basically remove the issue number, which was required by an now outdated rule. You also need to submit a changelog entry so our users can learn about your change. (This is a new requirement since last Septembe to speed up releasing.) |
@tallforasmurf pinging |
@tallforasmurf :-((((( |
@tallforasmurf So I did this work for you to get this finished at last. Please check the changelog-entries wither the description is correct. |
The dest path at this point is always a temp copy of the final exe; it is not helpful to the user to know it.
The code changes are I believe complete for this issue. There are problems with the manual, but I don't want to put doc changes into this pull request. After it is accepted I will open a new issue specifically to better document these various command parameters and their related spec file values.