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

delete unnecessary testproject and broken test #6494

Merged
merged 2 commits into from Sep 12, 2018

Conversation

Projects
None yet
2 participants
@cosmicexplorer
Copy link
Contributor

cosmicexplorer commented Sep 12, 2018

Problem

Spent a bit more time thinking after #6493 and there's no reason we need to test the case of a pants_requirement() in a python_dist()'s setup_requires -- as @jsirois said in #6490, these are both well-tested at lower levels, and the increase in CI time alone, even if it worked, is enough to make this removal-worthy.

Solution

  • Delete examples/src/python/example/python_distribution/hello/pants_setup_requires/ and test_pants_requirement_setup_requires_version, which is the broken test that depends on it.

Result

This was intentionally separated from #6493 so we can have absolutely no fear of merging that first to fix CI. Resolves #6490.

@cosmicexplorer cosmicexplorer requested review from jsirois and ity Sep 12, 2018

@jsirois
Copy link
Member

jsirois left a comment

Yay!

@cosmicexplorer cosmicexplorer force-pushed the cosmicexplorer:delete-pants-setup-requires branch from 42b95d7 to 7f731b7 Sep 12, 2018

@cosmicexplorer

This comment has been minimized.

Copy link
Contributor

cosmicexplorer commented Sep 12, 2018

Rebased after merging #6493, waiting for ci to go green again.

cosmicexplorer added some commits Sep 12, 2018

@cosmicexplorer

This comment has been minimized.

Copy link
Contributor

cosmicexplorer commented Sep 12, 2018

The unittest import in the python_dist integration test is now unused (it was only added to skip the test). The only reason I mention it is that I'm pretty sure I didn't disable or comment out the pre-commit hook, which should have caught that. I haven't been able to repro it.

Actually, I think it's because the pre-commit hook doesn't seem to run when editing an intermediate commit during a rebase. Is that true? This page on the git site doesn't answer the question but could imply by omission that pre-commit would run on every commit.

It's not important, I just can't see at all how that could have gotten past the pre-commit hook, I don't recall having to comment it out or remove it at all today.

@cosmicexplorer cosmicexplorer merged commit 6ff3842 into pantsbuild:master Sep 12, 2018

1 check passed

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