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.

Sign up for GitHub

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

Jump to bottom

Deprecate werkzeug.script #1090

Merged
merged 6 commits into from
Mar 30, 2017
Merged

Deprecate werkzeug.script #1090

merged 6 commits into from
Mar 30, 2017

Conversation

aaossa
Copy link
Contributor

@aaossa aaossa commented Mar 28, 2017 •
edited
Loading

This PR is directly related to #1079: "Fully deprecate werkzeug.script". These items must be solved before merge:

  • In werkzeug/contrib/profiler.py the make_action method is related to the deprecated module. Should we drop the method, drop profiler.py or leave it? SOLVED: Leave it 🙌
  • In each manage-X.py in the examples folder, werkzeug.script is imported (see manage-coolmagic.py for instance). We could replace it using click and explaining what click does in examples/README. Should I add it in this PR or another? SOLVED: 8098d03
  • In docs and comments I've been using "pallets/click" to make reference to the click module. Is this ok or should I use click instead? SOLVED: e722cb7

Waiting for your input 👌

@aaossa
Drop werkzeug.script from main code
c7a3841 
This commit deletes werkzeug/script.py, suggest using pallets/click
instead of werkzeug.script for bigger applications and deletes script
module from being loaded in werkzeug/__init__.py

Signed-off-by: Antonio Ossa <[email protected]>
@aaossa
Suggest pallets/click to replace werkzeug.script
e609041 
As suggested in the corresponding issue, click is the recommended
replacement

#1079 (comment)

Signed-off-by: Antonio Ossa <[email protected]>
@aaossa
Add deprecation note to CHANGES
cfc58ee 
Signed-off-by: Antonio Ossa <[email protected]>
@untitaker
Copy link
Contributor

In werkzeug/contrib/profiler.py the make_action method is related to the deprecated module. Should we drop the method, drop profiler.py or leave it?

I don't think it's very werkzeug.script-specific, we should leave it in. We don't really have a choice since we never deprecated it.

In each manage-X.py in the examples folder, werkzeug.script is imported (see manage-coolmagic.py for instance). We could replace it using click and explaining what click does in examples/README. Should I add it in this PR or another?

Yes please, in this PR.

In docs and comments I've been using "pallets/click" to make reference to the click module. Is this ok or should I use click instead?

In either case please link to http://click.pocoo.org when introducing it.

ThiefMaster
ThiefMaster reviewed Mar 28, 2017
View reviewed changes
docs/transition.rst Outdated
@@ -63,7 +63,7 @@ suggesting alternatives even if they will stick around for a longer time.
Do not use:

- `werkzeug.script`, replace it with custom scripts written with
`argparse` or something similar.
`argparse`, pallets/click or something similar.

This comment was marked as off-topic.

Sign in to view

@aaossa
Update examples
8098d03 
Every manage-<example>.py file used werkzeug.script a deprecated (and
now definitely removed) module. The new cli are powered by click [1]

[1]: https://github.com/pallets/click

Signed-off-by: Antonio Ossa <[email protected]>
@aaossa aaossa changed the title Deprecate werkzeug.scripts Deprecate werkzeug.script Mar 29, 2017
@aaossa
Recommend click to replace werkzeug.script
629b6a4 
This commit updates docs and comments to recommend `click` and display
the `click` website ([1]) when possible

[1]: http://click.pocoo.org

Signed-off-by: Antonio Ossa <[email protected]>
@aaossa
Copy link
Contributor Author

aaossa commented Mar 29, 2017

Travis says everything is ok 🙌 Can you please review this before merge @untitaker @ThiefMaster ? 😁 Thanks

@untitaker
Copy link
Contributor

Yeah those examples are good. However, the Travis tests don't cover them at all. Did you get those running? Otherwise LGTM

@aaossa
Update examples code
46bbf0a 
Some changes were made in `couchdb` and `werkzeug`. It's time to
update our examples. Now they should work on Python 2.

Signed-off-by: Antonio Ossa <[email protected]>
@aaossa
Copy link
Contributor Author

aaossa commented Mar 29, 2017

@untitaker Almost all of them work, except for:

  • i18nurls: Uses werkzeug.templates, and it seems like it was deprecated a while ago. (Maybe Jinja could be used)

I just tried using Python 2 (2.7.10), because most of the examples are not compatible with Python 3. Maybe in the future the examples could be updated to be compatible with Python 3 too.

@untitaker untitaker merged commit ee959e2 into pallets:master Mar 30, 2017
@untitaker
Copy link
Contributor

Thanks! The other stuff should be a separate PR. Ideally we'd have simple tests for each example like in Flask.

@davidism davidism added this to the 0.13 milestone Dec 5, 2017
This was referenced Dec 7, 2017
Closed
Merged
Merged
Closed
Merged
Closed
Merged
Merged
Merged
Closed
Merged
Closed
Merged
Merged
Closed
This was referenced Jan 5, 2018
Merged
Closed
Open
Closed
Open
Closed
Merged
Open
Closed
Closed
Closed
Closed
Closed
Open
This was referenced Jan 15, 2018
Closed
Closed
@pyup-bot pyup-bot mentioned this pull request Jan 22, 2018
Closed
This was referenced Jan 29, 2018
Closed
Closed
This was referenced Feb 5, 2018
Closed
Closed
Closed
Closed
This was referenced Feb 13, 2018
Closed
Closed
Closed
This was referenced Feb 26, 2018
Closed
Closed
@pyup-bot pyup-bot mentioned this pull request Mar 5, 2018
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ThiefMaster ThiefMaster ThiefMaster left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.13
Development

Successfully merging this pull request may close these issues.
4 participants
@aaossa @untitaker @ThiefMaster @davidism