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 dev-requirements.txt in MANIFEST.in. Issue #2246 #2286

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marston19
Copy link

Addresses #2246 - Include dev-requirements.txt in MANIFEST.in so sdist packaging get the necessary requirements to install and run tests.

Include dev-requirements.txt in MANIFEST.in so sdist packaging can make use of test requirements.
@bskinn
Copy link
Contributor

bskinn commented Aug 22, 2023

@mtelka, can you test your packaging workflow on @marston19's feature branch to make sure the only thing missing in the manifest was dev-requirements.txt?

@mtelka
Copy link

mtelka commented Aug 23, 2023

I tried to run tests for 3.3.1 both github tarball and PyPI sdist and test results are same. So the only difference in my use case is missing dev-requirements.txt file. Sorry, I cannot test the branch because I have no sdist from it. I can try to build it myself, but IIRC the result highly depends on tooling used to build sdist.

Thank you.

@bskinn
Copy link
Contributor

bskinn commented Aug 23, 2023

Sorry, I cannot test the branch because I have no sdist from it.

Ahh, of course, you need the sdist as it would be built from the project side.

I'll look into this and see what I can do for you.

@bskinn
Copy link
Contributor

bskinn commented Aug 23, 2023

Although, looking at the CircleCI output, it did include dev-requirements.txt in the manifest when it built.

So, the easiest (albeit slow) approach might be to merge and release this MANIFEST.in change, and then see how it works. If the MANIFEST still isn't right, we can fix further.

@mtelka
Copy link

mtelka commented Aug 23, 2023

That sounds like a plan :-). Please note that I do not need new paramiko release with this fix only. For the time being I just switched to github tarball.

Thank you!

@mtelka
Copy link

mtelka commented Aug 23, 2023

While talking about this... If you are open to make my life even more easier, then something like this would be highly appreciated.

Thank you!

@bskinn
Copy link
Contributor

bskinn commented Aug 23, 2023

While talking about this... If you are open to make my life even more easier, then something like this would be highly appreciated.

That one would be bitprophet's call.

Could you create a separate PR for adding the stub tox.ini and other related changes? It's a separate question from this ticket, and having it split out will make it easier to review/evaluate.

@mtelka mtelka mentioned this pull request Aug 23, 2023
@mtelka
Copy link

mtelka commented Aug 23, 2023

Could you create a separate PR for adding the stub tox.ini and other related changes? It's a separate question from this ticket, and having it split out will make it easier to review/evaluate.

Here it is: #2288

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

Successfully merging this pull request may close these issues.

None yet

3 participants