Skip to content

Conversation

@TallGirlVanessa
Copy link

Also add a more detailed explanation of how the request context and app
context interact.

Also add a more detailed explanation of how the request context and app
context interact.
@TallGirlVanessa
Copy link
Author

@jeffwidman
Copy link
Contributor

+1 for merging this as it'd cut down on bug reports hopefully. Probably a good idea to merge into master, in addition to 0.10 maintenance. Perhaps @davidism or @untitaker could review it.

@untitaker
Copy link
Contributor

@jeffwidman I invited you as member of pallets, feel free to do this one yourself!

@jeffwidman
Copy link
Contributor

Thanks. I'll cherrypick this into master this evening when I have more time.

@untitaker are there any guidelines for what gets merged into maintenance branches? There have been many docs changes over the past few years that could have been merged into the 0.10 maintenance branch but were only merged into master... not sure if this also should only go into master and not maintenance.

@untitaker
Copy link
Contributor

@jeffwidman Generally we only merge doc improvements of any kind into master, since the maintenance branches are really only for actual bugfixes in the code.

@TallGirlVanessa
Copy link
Author

@untitaker Put it this way: how can we make doc improvements that folks can see when they go to flask.pocoo.org and click "Docs"?

@untitaker
Copy link
Contributor

@garaden When the next release (0.11, 1.0, 1.1, 1.2) is out, these changes will be published there. The reason is that we don't want to represent features in the docs that are only in master, not any stable version.

@TallGirlVanessa
Copy link
Author

@untitaker Right, that makes total sense. It'd be super confusing if stable docs referenced features that aren't in the stable version.

That said, I still think this PR belongs in the stable docs. The app context teardown functionality is already present, documented, and widely used, in the current stable release (0.10.x). This PR fixes an omission in that existing documentation: I think there are plenty of Flask 0.10.x users who don't know how much trouble can be caused by allowing a teardown function to raise, or what to do to prevent it. In that sense, I think it is a bugfix, even if it's not in the actual code and doesn't result in a new stable version.

@untitaker
Copy link
Contributor

I don't consider documentation improvements of any kind to be bugfixes that need to be released as a bugfix release. Especially considering this PR does more than change a simple incorrect statement.

@untitaker
Copy link
Contributor

@garaden Another aspect to consider is: Links to stable documentation are made to be permanent (which is why when browsing the documentation, you always have the version number in the URL). If you quote something from the documentation, and link to a page of the 0.10 docs, and later the entire structure of the page changes, how would you feel?

@TallGirlVanessa
Copy link
Author

@untitaker It's definitely a grey area. If the documentation was completely wrong (like saying a parameter defaults to False when it actually defaults to True or something), I'd consider that worth a release. OTOH something like a typo is definitely not worth a release. I also agree a major change to the document structure, breaking links & stuff, would be bad. But I only added sections, I didn't remove or rename any, so existing links should be fine.

Ideally, I'd like to see this PR make it to the docs on flask.pocoo.org, without needing a tag or PyPI release. I was hoping that merging it to the maintenance branch, without tagging the merge commit, would accomplish this. Then again...I have to admit that's a weird thing to do. "All maintenance branch changes result in a new tag & release" sounds like a reasonable rule, and this PR breaks that rule.

Of course, this is your repo, not mine, so in the end the decision is yours :D I'm also glad to see an organization taking some of the weight of Flask & friends off of mitsuhiko. Major thanks for taking on that responsibility!

That also raises a question: off the record, given the repo changing hands, can we expect new Flask releases in the near future, say a year or so? I'd feel better about merging this only to master if I knew it would see stable release relatively soon.

TL;DR: Are there any next actions for me here? I can close this PR and reopen against master. Or just let y'all do the cherry-picking/merging and closing.

@jeffwidman
Copy link
Contributor

I can close this PR and reopen against master.

That would be much appreciated.

@TallGirlVanessa
Copy link
Author

Closing per @jeffwidman .

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants