Skip to content
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

A setup_py task for the new python pipeline. #4308

Merged
merged 2 commits into from Mar 6, 2017

Conversation

benjyw
Copy link
Sponsor Contributor

@benjyw benjyw commented Mar 6, 2017

Copied from the old one, with the previous special-casing of
codegen removed (the new python tasks don't invoke
codegen directly like the old ones did).

However this task does still require some special handling of
codegen: when selecting the targets to operate on we have to
replace any codegen target roots with their corresponding
synthetic targets, as those are the ones with the actual sources
to publish.

This change also copies the old task's unit tests, except the
ones pertaining to codegen (which would no longer work without
setting up and running codegen tasks in the tests).
Instead, this change adds an integration test that ensures that we
create an appropriate sdist from codegen targets, thus exercising
the special handling mentioned above (and incidentally also verifying
that our new pipeline does do the right thing regarding synthesizing
library targets for generated code).

Copied from the old one, with the previous special-casing of
codegen removed (since codegen targets are just producers of
source code products now, the new python tasks don't invoke
codegen directly like the old ones do).

However this task does still require some special handling of
codegen - when selecting the targets to operate on we have to
replace any codegen target roots with their corresponding
synthetic targets.

This change also copies the old task's unit tests, except the
ones pertaining to codegen, which would no longer work without
setting up and running codegen tasks.  Instead, this change
adds a regression test that ensures that we create an appropriate
sdist from codegen targets, thus exercising the special handling
mentioned above.
Copy link
Member

@kwlzn kwlzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

from pants_test.pants_run_integration_test import PantsRunIntegrationTest


class PythonReplIntegrationTest(PantsRunIntegrationTest):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Repl/SetupPy/ ?

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D'oh! Sucking at copypasta is a rare achievement indeed... :)

@benjyw benjyw merged commit f18c556 into pantsbuild:master Mar 6, 2017
@benjyw benjyw deleted the new_pipeline_setup_py branch March 6, 2017 22:03
lenucksi pushed a commit to lenucksi/pants that referenced this pull request Apr 25, 2017
Copied from the old one, with the previous special-casing of
codegen removed (the new python tasks don't invoke
codegen directly like the old ones did).

However this task does still require some special handling of
codegen: when selecting the targets to operate on we have to
replace any codegen target roots with their corresponding
synthetic targets, as those are the ones with the actual sources
to publish.

This change also copies the old task's unit tests, except the
ones pertaining to codegen (which would no longer work without
setting up and running codegen tasks in the tests).
Instead, this change adds an integration test that ensures that we
create an appropriate sdist from codegen targets, thus exercising
the special handling mentioned above (and incidentally also verifying
that our new pipeline does do the right thing regarding synthesizing
library targets for generated code).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants