Repository navigation
PEP8 and other issues #1
Description
Please fix them both in the example project here and in the online tutorial. :)
PEP8
- spaces around the
=of keywords args - PEP8 forbids spaces there - inconsistent indentation (some function bodies are indented with 8 spaces)
- local imports for no reason (
datetimeinhello_there) - not directly a PEP8 violation but local stdlib imports don't really make sense
Flask stuff
- please consider removing the example with the hardcoded HTML. some people WILL think it's OK to do that even if you mention that usually templates are used. also, that hardcoded-html example is vulnerable to XSS.
href="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/static/site.css"- usehref="{{ url_for('static', filename='site.css')}}"message="Hello there, " + name + "!",- usually you would pass just thenameto the template and then build the message in there. So I would change this, and also do thestrftimecall in the template (ideally someone would use e.g. flask-babel but that's out of the scope of a simple tutorial)
Note: I'm one of the flask maintainers, and I can assure you that people WILL assume things are good / ok / the way to go when part of a tutorial. I've seen my share of bad practices from code people posted in our IRC channel.
Reacted by Robert Grant
Activity
kraigb commentedon Jul 7, 2018
Thanks! I'll work on this early next week and I'll want to make sure the text surrounding the tutorial is updated as well.
kraigb commentedon Jul 11, 2018
Adrian (@ThiefMaster) Can you clarify why one would choose to put the strftime call in the template instead of the code?
ThiefMaster commentedon Jul 11, 2018
It's display logic -> more suitable in the template. (also, usually (ie in a bigger application) one would use an extension like flask-babel that registers Jinja filters to do it.)
kraigb commentedon Jul 11, 2018
Thanks. I primarily wanted to mention that reason and then also provide a link to flask-babel.
Updates per Adrian
kraigb commentedon Jul 11, 2018
Adrian (@ThiefMaster) Can you review #2 for me to make sure I've addressed your concerns in the code? I'll also ask for a review on the changes to the tutorial, but I want to get the code in place first.
update microsoft#1
Merge pull request microsoft#1 from denniseik/feature/azureml-workspace
Merge pull request microsoft#1 from ryhud/ryhud-301
Merge pull request microsoft#1 from lonegunmanb/UserStory1981979_zj
Set up CI with Azure Pipelines microsoft#1