Skip to content

Fix an issue with setup scripts in tox.ini.#9

Closed
craigcitro wants to merge 1 commit intogoogle:masterfrom
craigcitro:cleanup_tox
Closed

Fix an issue with setup scripts in tox.ini.#9
craigcitro wants to merge 1 commit intogoogle:masterfrom
craigcitro:cleanup_tox

Conversation

@craigcitro
Copy link
Copy Markdown
Contributor

This tweaks tox.ini to install additional packages in such a way that we
don't run afoul of the issues in #4.

@tseaver do you have any sense for why this would fix the failures we're seeing in #4? this is far beyond my python-packaging-fu. in particular, danny found this link with the fix, but I'm still confused as to why this particular fix works, whereas other attempts (including not calling pip from inside tox.ini) didn't.

This tweaks `tox.ini` to install additional packages in such a way that we
don't run afoul of the issues in google#4.
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 39.85% when pulling 4912fa5 on craigcitro:cleanup_tox into c431900 on google:master.

@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Mar 31, 2015

@craigcitro the first example uses the [testing] extras from the PyPI version (rather than the local checkout).

I think I would probably just punt on the DRY and inline the list into tox.ini -- anybody who is likely to care will be running tox anyhow.

@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Mar 31, 2015

The Travis 3.4 failure is likely due to hash order stuff: pass sort_keys=True to json.dumps() to normalize, or parse them back to dicts to compare.

@craigcitro
Copy link
Copy Markdown
Contributor Author

@tseaver i had thought the same thing about dropping the pip line, but @dhermes said it failed when he tried. that leaves me utterly confused. 😉

i'll fix the sort_keys thing as soon as i get a sec.

@tseaver
Copy link
Copy Markdown
Contributor

tseaver commented Mar 31, 2015

My take on a better fix is in #10.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants