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

Add the support and the tests of Champlain and Clutter #3443

Merged
merged 3 commits into from Sep 4, 2018

Conversation

Projects
None yet
2 participants
@benoit9126
Contributor

benoit9126 commented Apr 4, 2018

Hi,

I used PyInstaller to freeze an application which needed Clutter, GtkClutter, Champlain and GtkChamplain. I have written some basic hooks for that and I propose to add them in PyInstaller.

I also modified the test suite of gi hooks to be able to handle the weird version number of Champlain and GtkChamplain (0.12)

@benoit9126

This comment has been minimized.

Contributor

benoit9126 commented Apr 4, 2018

A build on travis errorred but it is not related to my modifications (https://travis-ci.org/pyinstaller/pyinstaller/jobs/362088314). Could you please restart the job? I can not do it myself...

@htgoebel

This comment has been minimized.

Member

htgoebel commented Apr 8, 2018

Thanks for the pull-request. Can you please split the changes in tests/functional/test_hooks/test_gi.py unrelated to the new hook into a commit of it's own. Thanks.

When updating a pull-request, you can simply (force) push the updated branch to github again. This will automatically update the pull-request (which follows the branch, not the commit). So you do not need to close the pull-request and open a new one. This also has the benefit that the discussion history is kept. For detailed instructions please read Updating a Pull-Request in the manual.

Please also take the change and rebase your branch on the current develop head. Thanks.

@htgoebel htgoebel added the hooks label Apr 8, 2018

@benoit9126

This comment has been minimized.

Contributor

benoit9126 commented Apr 9, 2018

It should do the job.

@htgoebel

This comment has been minimized.

Member

htgoebel commented Apr 13, 2018

Thanks for the update. The second commit does more than the commit message says. It massively reworks the test-case and adds test. Thus this should become two commits.

Please follow the detailed commit guideline and the guide for good commit messages. Mind to prefix with the subsystem ("Hooks" resp. "Tests"). Se the later links for why we are so picky about this.

@benoit9126

This comment has been minimized.

Contributor

benoit9126 commented Apr 16, 2018

@htgoebel : You are right. Here is the modification.

PS: Sorry, I was in trouble with Git and some useless continuous integration tests may have been triggered...

@benoit9126

This comment has been minimized.

Contributor

benoit9126 commented Apr 17, 2018

It seems that the -n option of py.test is not known:

$ py.test -n 3 --maxfail 3 tests/unit tests/functional --ignore=tests/functional/test_libraries.py --ignore=tests/functional/test_hooks
usage: py.test [options] [file_or_dir] [file_or_dir] [...]
py.test: error: unrecognized arguments: -n tests/unit tests/functional
  inifile: /home/travis/build/pyinstaller/pyinstaller/setup.cfg
  rootdir: /home/travis/build/pyinstaller/pyinstaller
The command "py.test -n 3 --maxfail 3 tests/unit tests/functional --ignore=tests/functional/test_libraries.py --ignore=tests/functional/test_hooks
" exited with 2.

Benoît Vinot added some commits Apr 9, 2018

Benoît Vinot
Benoît Vinot
Benoît Vinot

@htgoebel htgoebel merged commit 59aac75 into pyinstaller:develop Sep 4, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@htgoebel

This comment has been minimized.

Member

htgoebel commented Sep 4, 2018

Thanks you for this pull-request, which is a nice one :-) I'm sorry to not have merged it earlier.

@htgoebel htgoebel added this to the PyInstaller 3.4 milestone Sep 4, 2018

@benoit9126

This comment has been minimized.

Contributor

benoit9126 commented Sep 5, 2018

No problem. ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment