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

Include contrib module in installed package #5

Merged
merged 3 commits into from Mar 3, 2015

Conversation

blaix
Copy link
Contributor

@blaix blaix commented Mar 2, 2015

@thomasw
Copy link

thomasw commented Mar 2, 2015

👍

@zoidyzoidzoid
Copy link

version bump and changelog entry?

@zoidyzoidzoid
Copy link

I was gonna retract my statement, but it is on YolaPI. So it should still need the version bump.

@blaix
Copy link
Contributor Author

blaix commented Mar 3, 2015

Yeah, I usually like to do those in a separate pull, but this change is all about the released package, so I'm going to add it to this one. Stand by...

@blaix
Copy link
Contributor Author

blaix commented Mar 3, 2015

Bumped. Can I get a new thumb?

@adrianmoisey
Copy link
Contributor

👍

blaix added a commit that referenced this pull request Mar 3, 2015
Include contrib module in installed package
@blaix blaix merged commit 3edf4e4 into master Mar 3, 2015
@blaix blaix deleted the include_contrib_package branch March 3, 2015 10:51
@zoidyzoidzoid
Copy link

Yeah, some people think it should be another pull, and some people think version bumps and changelog changes should happen in master.

Generally our client libraries get one pull of changes per version bump, so I'm not against them going in the same pull.

I should've commented saying that it's okay if you were gonna do it in another PR or somewhere else.

@stefanor
Copy link
Contributor

stefanor commented Mar 3, 2015

I don't think we want a global contrib package like that. It should probably be inside proxyprefix, e.g. proxyprefix.contrib.

Otherwise what happens if two libraries ship a contrib package?

@blaix
Copy link
Contributor Author

blaix commented Mar 3, 2015

It is proxyprefix.contrib...

@blaix
Copy link
Contributor Author

blaix commented Mar 3, 2015

Oops, yeah. My CHANGELOG entry is wrong. I'll fix it.

blaix added a commit that referenced this pull request Mar 3, 2015
@blaix
Copy link
Contributor Author

blaix commented Mar 3, 2015

#6

@stefanor
Copy link
Contributor

stefanor commented Mar 3, 2015

In that case, why did you need to switch to find_packages?

@thomasw
Copy link

thomasw commented Mar 3, 2015

From https://docs.python.org/2/distutils/setupscript.html#listing-whole-packages

(Keep in mind that although package_dir applies recursively, you must explicitly list all packages in packages: the Distutils will not recursively scan your source tree looking for any directory with an init.py file.)

I've read this like 4 times, and I'm still not sure my interpretation is correct. If I understand things correctly package_dir can be used in addition to or instead of packages? package_dir will work recursively but packages does not? I've never tried to use package_dir, so I really don't know the answer here. If package_dir were used here, would the parameter be package_dir={'proxyprefix':'proxyprefix'}? 🎱

Justin made this change because proxyprefix.contrib was not being included in the dist.

@stefanor
Copy link
Contributor

stefanor commented Mar 4, 2015

Right, I always forget about that. This is why find_packages exists...

@zoidyzoidzoid
Copy link

I remember going through this confusion with healthcheck too.

On Wed, Mar 4, 2015 at 9:53 AM, Stefano Rivera notifications@github.com
wrote:

Right, I always forget about that. This is why find_packages exists...


Reply to this email directly or view it on GitHub
#5 (comment).

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

5 participants