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

Altgraph: revert upstream patches #3058

Merged
merged 4 commits into from Mar 13, 2018

Conversation

Projects
None yet
1 participant
@ghost

ghost commented Dec 1, 2017

xref #1231.

Essentially this PR attempts to revert upstream patches so that we can delete the altgraph files. Tests are expected to fail for Python 3.3.

xoviat added some commits Dec 1, 2017

xoviat
xoviat
@htgoebel

This comment has been minimized.

Member

htgoebel commented Dec 1, 2017

What is the aim of this pull-request?

@ghost

This comment has been minimized.

ghost commented Dec 1, 2017

FIXME: This behaviour differs from upstream modulegraph

@ghost ghost closed this Dec 1, 2017

@ghost ghost reopened this Dec 1, 2017

xoviat added some commits Dec 2, 2017

xoviat
xoviat

@ghost ghost changed the title from WIP: Altgraph: revert upstream patches to Altgraph: revert upstream patches Dec 2, 2017

@htgoebel

This comment has been minimized.

Member

htgoebel commented Dec 2, 2017

FIXME: This behaviour differs from upstream modulegraph

Where? I really love you setting up puzzles. NOT!

@ghost

This comment has been minimized.

ghost commented Dec 2, 2017

@htgoebel

This comment has been minimized.

Member

htgoebel commented Dec 6, 2017

IC

@htgoebel htgoebel added this to the PyInstaller 3.4 milestone Dec 7, 2017

@htgoebel htgoebel added the modulegraph label Dec 7, 2017

@htgoebel

This comment has been minimized.

Member

htgoebel commented Dec 15, 2017

To summarize: This reverts 49c725e, thus altgraph becomes the same as upstream version and thus we can use upstream. Correct?

Does it make sense to keep the altgraph test-suite then? What do you think?

@ghost

This comment has been minimized.

ghost commented Dec 15, 2017

Yes. If the upstream API changes, we need to know about it.

@ghost

This comment has been minimized.

ghost commented Dec 15, 2017

Also, these tests are trivial to run. I can understand being hesitant to add tests for hooks, but there's no reason not to keep these.

@htgoebel

This comment has been minimized.

Member

htgoebel commented Dec 15, 2017

Fine for me.

@htgoebel htgoebel merged commit 6c25ec2 into pyinstaller:develop Mar 13, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment