Improve "introduction" page, add "Advanced Features" page #2634
Conversation
|
@stevepiercy can you help me figure out what I've done to break the build on this PR? And if you have time, some feedback on the direction I'm taking with these changes would be deeply appreciated. |
|
@cewing I checked the build history https://ci.appveyor.com/project/mmerickel/pyramid/history but there is no record of the attempt to build when you made your last commit. I'd say it was just a temporary thing that might resolve itself with your next commit. |
|
Looks like something went wrong with Appveyor, it seems to think this is an un-mergeable pull request. |
|
I'll rebase on the changes from master and hopefully that will clear it up. As it is a WIP, I'm not too worried for the time being, but it'd be nice not to have too big a mess to clean up once the work is done :) |
|
@cewing I forgot to mention, I like the direction you are headed. Active headings, simpler phrasing... all good stuff. |
|
@cewing Wouldn't rebasing against master be cleaner than a merge which adds 168 commits? ;) |
|
My bad. I told him to just merge. |
|
OK. I sometimes find commit-by-commit review easier, and so try not to do big merges on "reviewable" branches. |
|
Don't worry, @tseaver, I'll clean up before I ask anyone to review this. |
|
This PR is now ready for review. In short, here's what I've done:
I'm happy to make whatever adjustments are requested. |
|
@cewing we're gonna need a bigger 🍺 . I'll look this over tonight with review comments, if any. Thank you! |
docs/narr/advfeatures.rst
Outdated
| You Don't Need Singletons | ||
| ------------------------- | ||
|
|
||
| Have you ever struggled with parametrizing Django's ``settings.py`` file for |
docs/narr/advfeatures.rst
Outdated
| Have you ever struggled with parametrizing Django's ``settings.py`` file for | ||
| multiple installations of the same Django application? Have you ever needed to | ||
| monkey-patch a framework fixture to get it to behave properly for your | ||
| use-case? Have you ever tried to deploy your application using an asynchronous |
docs/narr/advfeatures.rst
Outdated
| server and failed? | ||
|
|
||
| All these problems are symptoms of "mutable global state", also known as | ||
| "import-time side effects" and arise from the use of "singleton" data structures. |
There was a problem hiding this comment.
When I read this, I see someone making "air quotes". Seriously, though, these are undefined terms, and will fly over folks' heads unless they are defined somewhere. The terms should be linked to their definitions. Alternatively drop the air quotes and assume that the reader knows what these terms mean.
There was a problem hiding this comment.
I've added some of these to the glossary and converted them to :term: references
docs/narr/advfeatures.rst
Outdated
| if the request method is GET, another view if the request method is POST, and | ||
| so on. | ||
|
|
||
| :app:`Pyramid` uses a system of "view predicates" to allow this. Matching the |
There was a problem hiding this comment.
No air quotes. A link to view predicates either in the docs or glossary would be better.
docs/narr/advfeatures.rst
Outdated
| query string, the Accept header, whether the request is an XHR request or not, | ||
| and lots of other things. | ||
|
|
||
| For our example above, you can do this instead:: |
There was a problem hiding this comment.
Please use explicit .. code-block:: language on a new line after a single :, instead of implicit ::. This helps ensure that the code has valid language syntax and is highlighted with the proper Pygments lexer.
docs/narr/introduction.rst
Outdated
| Mistakes happen. Problems crop up. No-one writes bug-free code. Pyramid | ||
| provides a way to handle the exceptions your code encounters. An | ||
| :term:`exception view` is a special kind of view which is automatically called | ||
| when an particular exception type "bubbles up" without being handled by your |
There was a problem hiding this comment.
when a particular
also s/"bubbles up"/arises
docs/narr/introduction.rst
Outdated
| For example, you might register an exception view for the :exc:`Exception` | ||
| exception type, which will catch *all* exceptions, and present a pretty "well, | ||
| this is embarrassing" page. Or you might choose to register an exception view | ||
| for only certain application-specific exceptions. You can make a one for when a |
docs/narr/advfeatures.rst
Outdated
| @@ -0,0 +1,399 @@ | |||
| Advanced :app:`Pyramid` Design Features | |||
There was a problem hiding this comment.
I'd like the filename to be spelled out, "advanced-features". See below, too.
docs/narr/introduction.rst
Outdated
| trolls" or other people who seem to get their rocks off by berating fellow | ||
| users in our various official support channels. We try to keep it well-lit and | ||
| new-user-friendly. | ||
| advfeatures |
docs/narr/introduction.rst
Outdated
| nor discouraging the decision. | ||
| Similar to :term:`Zope`, :app:`Pyramid` applications may easily be extended. If | ||
| you work within the constraints of the framework, you can produce applications | ||
| that can be reused, modified or extended without needing to modify the original |
There was a problem hiding this comment.
reused, modified, or extended (Oxford comma)
|
Thanks for the careful reading, @stevepiercy. Working on fixes now. |
|
@stevepiercy, I've completed fixes to the advanced features file per your code review. I've also renamed the file as you requested, which seems to have marked all the diffs as out-of-date. You might take a final look at that file before giving it the go-ahead. I'll finish the intro text CR fixes tomorrow or soon. |
docs/narr/introduction.rst
Outdated
| and 95% decision/condition coverage. (as measured by `instrumental <http://instrumental.readthedocs.io/en/latest/intro.html>`_) | ||
| It is automatically tested using `Travis <https://travis-ci.org/Pylons/pyramid>`_ | ||
| and `Jenkins <http://jenkins.pylonsproject.org/job/pyramid/>`_ | ||
| on supported versions of Python after each commit to its GitHub repository. |
There was a problem hiding this comment.
Although Plone docs use semantic line breaks, that went over like a lead balloon with Pylons Project core developers: #2633
One line per sentence was kinda sorta OK.
One line per paragraph in .rst only is preferred.
Otherwise the changes look good.
docs/narr/advanced-features.rst
Outdated
|
|
||
| if __name__ == '__main__': | ||
| config = Configurator() | ||
| config.add_response_adapter(string_response_adapter, basestring) |
There was a problem hiding this comment.
str, the docs are focused on python 3 over python 2.
docs/narr/advanced-features.rst
Outdated
|
|
||
| if __name__ == '__main__': | ||
| config = Configurator() | ||
| config.add_response_adapter(string_response_adapter, basestring) |
There was a problem hiding this comment.
str, the docs are focused on python 3 over python 2.
|
Okay. @mmerickel, @stevepiercy, this should be ready to go, unless you have more requested changes. I think I've addressed everything from your reviews. Thanks for the thoughtful and close reading! c |
cewing
changed the title
There was a problem hiding this comment.
These change requests should be quick. A few changes got overlooked from my original review, so I've collected them all in a single review here.
As an FYI for future PRs regarding line lengths and breaks, I updated this on my style-guide branch. I won't hold back this PR for line breaks/lengths here because your work is herculean and I am experimenting with running all docs through a post-processor anyway, pre-release of 1.9-branch.
docs/narr/advanced-features.rst
Outdated
| But if you're a developer who likes the aesthetics of simplicity, Pyramid | ||
| provides an way to support this sort of thing, the *response adapter*: | ||
| But if you're a developer who likes the aesthetics of simplicity, | ||
| :app:`Pyramid` provides an way to support this sort of thing, the |
docs/narr/advanced-features.rst
Outdated
| ---------------------------------------- | ||
|
|
||
| Speaking of the :app:`Pyramid` structured "include" mechanism (see | ||
| :meth:`~pyramid.config.Configurator.include`), it allows you to compose complex |
There was a problem hiding this comment.
Let's condense these lines:
structured :meth:`~pyramid.config.Configurator.include` mechanism, it allows you to compose complex
docs/narr/advanced-features.rst
Outdated
|
|
||
| But if you're a developer who likes the aesthetics of simplicity, | ||
| :app:`Pyramid` provides an way to support this sort of thing, the | ||
| :term:`response adapter`\ : |
docs/narr/introduction.rst
Outdated
| demonstrations of common scenarios you might face. Contributions in the form of | ||
| improvements to our documentation are always appreciated. And we always welcome | ||
| improvements to our `official tutorials | ||
| <http://docs.pylonsproject.org/projects/pyramid/en/latest/#tutorials>`_ as well |
There was a problem hiding this comment.
I goofed. This should be:
:ref:`official tutorials <html_tutorials>`
docs/narr/introduction.rst
Outdated
|
|
||
| The :app:`Pyramid` documentation is comprehensive. We strive to keep our | ||
| narrative documentation both complete and friendly to newcomers. We also | ||
| maintain a :ref:`cookbook <cookbook:pyramid-cookbook>` of recipes, |
There was a problem hiding this comment.
maintain the :ref:`Pyramid Community Cookbook <cookbook:pyramid-cookbook>` of recipes,
docs/narr/introduction.rst
Outdated
| improvements to our `official tutorials | ||
| <http://docs.pylonsproject.org/projects/pyramid/en/latest/#tutorials>`_ as well | ||
| as new contributions to our `community maintained tutorials | ||
| <http://docs.pylonsproject.org/projects/pyramid-tutorials/en/latest/index.html#pyramid-tutorials>`_. |
There was a problem hiding this comment.
I goofed again, this time forgetting about Intersphinx links.
:ref:`community maintained tutorials <tutorials:pyramid-tutorials>`
docs/narr/introduction.rst
Outdated
| The :app:`Pyramid` documentation is comprehensive. We strive to keep our | ||
| narrative documentation both complete and friendly to newcomers. We also | ||
| maintain a :ref:`cookbook <cookbook:pyramid-cookbook>` of recipes, | ||
| demonstrations of common scenarios you might face. Contributions in the form of |
docs/narr/introduction.rst
Outdated
| at the top of the screen based on an enumeration of views they registered. | ||
|
|
||
| This is possible using Pyramid's :term:`introspector`. | ||
| Mistakes happen. Problems crop up. No-one writes bug-free code. :app:`Pyramid` |
docs/narr/introduction.rst
Outdated
| Mistakes happen. Problems crop up. No-one writes bug-free code. :app:`Pyramid` | ||
| provides a way to handle the exceptions your code encounters. An | ||
| :term:`exception view` is a special kind of view which is automatically called | ||
| when an particular exception type "bubbles up" without being handled by your |
There was a problem hiding this comment.
"exception type arises without"
There was a problem hiding this comment.
oh, and "a particular" :)
There was a problem hiding this comment.
good catch. Misteaks happen. 😛
docs/narr/introduction.rst
Outdated
| For example, you might register an exception view for the :exc:`Exception` | ||
| exception type, which will catch *all* exceptions, and present a pretty "well, | ||
| this is embarrassing" page. Or you might choose to register an exception view | ||
| for only certain application-specific exceptions. You can make a one for when a |
|
Okay, @stevepiercy, fixed those last few, found a couple more of my own, and reflowed both docs to use the one-line-per-paragraph style. |
There was a problem hiding this comment.
Just one tiny wafer-thin change, and I'm good. Ping @mmerickel to check whether his request for review has been satisfied.
docs/narr/introduction.rst
Outdated
| Documented | ||
| ~~~~~~~~~~ | ||
|
|
||
| The :app:`Pyramid` documentation is comprehensive. We strive to keep our narrative documentation both complete and friendly to newcomers. We also maintain the :ref:`Pyramid Community Cookbook <cookbook:pyramid-cookbook>` of ecipes demonstrating common scenarios you might face. Contributions in the form of improvements to our documentation are always appreciated. And we always welcome improvements to our `official tutorials <html_tutorials>`_ as well as new contributions to our `community maintained tutorials <tutorials:pyramid-tutorials>`_. |
There was a problem hiding this comment.
"of ecipes" is missing an "r".
|
heh. Reflow error. Fixed now. Have at it, @mmerickel! And thanks again for being a great editor, @stevepiercy |
|
Fantastic work @cewing ! |
|
Thank you! |
|
FWIW, this PR would have been a really good candidate for a squash merge. |
This is a work in progress, please do not merge.
refs #2614