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

Revise Quick Tutorial `setup.py`s #3380

Merged
merged 14 commits into from Oct 9, 2018

Conversation

Projects
None yet
4 participants
@stevepiercy
Member

stevepiercy commented Oct 8, 2018

No description provided.

stevepiercy added some commits Oct 8, 2018

Some steps are not the previous, but earlier.
Mention that we may now configure setup.py and run pip install variants.
Emphasize the changes to setup.py
Reformat numbered list.
Align emphasize-lines to setup.py
Reformat numbered list.

@stevepiercy stevepiercy self-assigned this Oct 8, 2018

@stevepiercy stevepiercy requested a review from mmerickel Oct 8, 2018

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Oct 8, 2018

Member

So the question that came up the other day is whether we should rename the testing extra to dev for apps? It is named testing in pyramid because we have real reasons as a library to split them apart into testing and docs (rtd only needs to install docs, not testing). However apps are pretty happy with just dev/prod dependencies, so if we rename the extra to dev_requires instead of tests_require and define a dev extra instead of testing maybe that will make more sense to people.

Does anyone disagree? I'm +1 on all the changes in this PR and I would be +1 on the renaming as well but am also fine if we keep it named testing.

Member

mmerickel commented Oct 8, 2018

So the question that came up the other day is whether we should rename the testing extra to dev for apps? It is named testing in pyramid because we have real reasons as a library to split them apart into testing and docs (rtd only needs to install docs, not testing). However apps are pretty happy with just dev/prod dependencies, so if we rename the extra to dev_requires instead of tests_require and define a dev extra instead of testing maybe that will make more sense to people.

Does anyone disagree? I'm +1 on all the changes in this PR and I would be +1 on the renaming as well but am also fine if we keep it named testing.

@stevepiercy stevepiercy added this to the 1.10 milestone Oct 8, 2018

@stevepiercy

This comment has been minimized.

Show comment
Hide comment
@stevepiercy

stevepiercy Oct 8, 2018

Member

FWIW, I have no problem doing the renaming.

I would much rather have a well-named extra than one that makes me go "wtf? pyramid_debugtoolbar is neither a production nor a testing requirement! I'm just trying to develop my first Pyramid app!"

Member

stevepiercy commented Oct 8, 2018

FWIW, I have no problem doing the renaming.

I would much rather have a well-named extra than one that makes me go "wtf? pyramid_debugtoolbar is neither a production nor a testing requirement! I'm just trying to develop my first Pyramid app!"

@blaflamme

This comment has been minimized.

Show comment
Hide comment
@blaflamme

blaflamme Oct 8, 2018

Member

I think tests or testing still makes more sense than dev regarding tests dependencies. I agree that while you're developing you normally need tests dependencies but dev is more a state of a project than tests for declaring a set of specific dependencies. As an example it makes more sense to install prod deps + tests deps for the CI before deploying to production than installing dev deps that hides tests deps for production. I still prefer to be explicit than implicit here.

Member

blaflamme commented Oct 8, 2018

I think tests or testing still makes more sense than dev regarding tests dependencies. I agree that while you're developing you normally need tests dependencies but dev is more a state of a project than tests for declaring a set of specific dependencies. As an example it makes more sense to install prod deps + tests deps for the CI before deploying to production than installing dev deps that hides tests deps for production. I still prefer to be explicit than implicit here.

@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Oct 8, 2018

Member

What I have seen from the packaging community is that there has been a real push for applications to focus only on dev and prod requirements and to not split it up into more fine-grained extras like testing, linting, publishing, etc. This is different for libraries imo, but for apps the stance has been pretty clear that the two categories satisfies most users needs. Bundler, NPM, and Cargo also have clearly gotten pretty far using that strategy. Note that both pipenv and poetry has taken this stance as well, although poetry does support extras as well as an escape hatch if needed.

Member

mmerickel commented Oct 8, 2018

What I have seen from the packaging community is that there has been a real push for applications to focus only on dev and prod requirements and to not split it up into more fine-grained extras like testing, linting, publishing, etc. This is different for libraries imo, but for apps the stance has been pretty clear that the two categories satisfies most users needs. Bundler, NPM, and Cargo also have clearly gotten pretty far using that strategy. Note that both pipenv and poetry has taken this stance as well, although poetry does support extras as well as an escape hatch if needed.

@blaflamme

This comment has been minimized.

Show comment
Hide comment
@blaflamme

blaflamme Oct 8, 2018

Member

I also agree about the difference between library and application. While I still prefer a fine-grained approach myself I can see the benefit of sharing what others are doing, less brain shifting. I have not started using pipenv, I'm still not sure about it bringing the best of all packaging worlds, poetry seems to be a good alternative.

Member

blaflamme commented Oct 8, 2018

I also agree about the difference between library and application. While I still prefer a fine-grained approach myself I can see the benefit of sharing what others are doing, less brain shifting. I have not started using pipenv, I'm still not sure about it bringing the best of all packaging worlds, poetry seems to be a good alternative.

@bertjwregeer

This comment has been minimized.

Show comment
Hide comment
@bertjwregeer

bertjwregeer Oct 8, 2018

Member

I personally have started using poetry for new projects and am all for dropping the extra distinction, it makes things cleaner. On CI I install develop, on production I install the final package without any extras.

I think just dev instead of testing is a great idea. 👍

Member

bertjwregeer commented Oct 8, 2018

I personally have started using poetry for new projects and am all for dropping the extra distinction, it makes things cleaner. On CI I install develop, on production I install the final package without any extras.

I think just dev instead of testing is a great idea. 👍

@stevepiercy

This comment has been minimized.

Show comment
Hide comment
@stevepiercy

stevepiercy Oct 8, 2018

Member

Is this the consensus?

Example before:

requires = [
    'bcrypt',
    'pyramid',
    'pyramid_chameleon',
    'pyramid_debugtoolbar',
    'waitress',
]

tests_require = [
    'pytest',
    'webtest',
]

 setup(
    name='tutorial',
    install_requires=requires,
    extras_require={
        'testing': tests_require,
    },
)

Example after:

requires = [
    'bcrypt',
    'pyramid',
    'pyramid_chameleon',
    'waitress',
]

dev_requires = [
    'pyramid_debugtoolbar',
    'pytest',
    'webtest',
]

 setup(
    name='tutorial',
    install_requires=requires,
    extras_require={
        'dev': dev_requires,
    },
)
Member

stevepiercy commented Oct 8, 2018

Is this the consensus?

Example before:

requires = [
    'bcrypt',
    'pyramid',
    'pyramid_chameleon',
    'pyramid_debugtoolbar',
    'waitress',
]

tests_require = [
    'pytest',
    'webtest',
]

 setup(
    name='tutorial',
    install_requires=requires,
    extras_require={
        'testing': tests_require,
    },
)

Example after:

requires = [
    'bcrypt',
    'pyramid',
    'pyramid_chameleon',
    'waitress',
]

dev_requires = [
    'pyramid_debugtoolbar',
    'pytest',
    'webtest',
]

 setup(
    name='tutorial',
    install_requires=requires,
    extras_require={
        'dev': dev_requires,
    },
)
@mmerickel

This comment has been minimized.

Show comment
Hide comment
@mmerickel

mmerickel Oct 9, 2018

Member

LGTM

Member

mmerickel commented Oct 9, 2018

LGTM

@bertjwregeer

This comment has been minimized.

Show comment
Hide comment
@bertjwregeer

bertjwregeer Oct 9, 2018

Member
Member

bertjwregeer commented Oct 9, 2018

@stevepiercy

This comment has been minimized.

Show comment
Hide comment
@stevepiercy

stevepiercy Oct 9, 2018

Member

Changes made. Ready for final review.

Member

stevepiercy commented Oct 9, 2018

Changes made. Ready for final review.

@stevepiercy stevepiercy merged commit 0591572 into Pylons:master Oct 9, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stevepiercy stevepiercy deleted the stevepiercy:docs-quick-tutorial-update branch Oct 9, 2018

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