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.

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

migrate pyramid to use plaster #2985

Merged
merged 7 commits into from
Apr 30, 2017
Merged

migrate pyramid to use plaster #2985

merged 7 commits into from
Apr 30, 2017

Conversation

mmerickel
Copy link
Member

@mmerickel mmerickel commented Mar 28, 2017

  • Replace low-level functions with plaster.
  • Update proutes.
  • Update pserve.
  • Update pshell.
  • Update logging chapter.
  • Update PasteDeploy chapter.
  • Update cookiecutters to depend on plaster_pastedeploy.

Fixes #2419, #1394.

@stevepiercy stevepiercy modified the milestone: 1.8 Mar 28, 2017
@stevepiercy
Copy link
Member

What's the milestone/target version for these items?

@mmerickel mmerickel added this to the 1.9 milestone Mar 28, 2017
@mmerickel mmerickel changed the title rewrite low-level pyramid config functions to use plaster migrate pyramid to use plaster Mar 30, 2017
@mmerickel
Copy link
Member Author

At least for now I've updated pyramid to depend on plaster_pastedeploy which makes this PR bw-compatible. Shipping pyramid without any specific config-file loader is something we can debate. It's a pretty substantial amount of stuff, even just in core, that needs fixes if we rip out that dependency.

@mmerickel
Copy link
Member Author

I think I'm done with this for now, pending review. I updated a couple locations in the documentation but tried not to make too big of a deal about it at this time. It's a new feature and there's no sense in jamming it down people's throats so I just tried to give some clues that pastedeploy could be replaced.

Copy link
Member

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in docs. There's only a few minor syntax and grammar errors.

CHANGES.txt Outdated
and ``pshell``, as well as the ``pyramid.paster.bootstrap`` function
is now replaceable thanks to a new dependency on
`plaster <http://docs.pylonsproject.org/projects/plaster/en/latest/>`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rst links require a trailing underscore:

`label <link>`_

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks I pushed some changes.

CHANGES.txt Outdated
The first library to use this feature is
`pyramid_retry
<http://docs.pylonsproject.org/projects/pyramid-retry/en/latest/>`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add trailing _

CHANGES.txt Outdated
objects before they enter rest of the pipeline. This means for a given
request that the policy may create more than one request for retry
purposes. See https://github.com/Pylons/pyramid/pull/2964
objects before they enter rest of the pipeline. This means for a single
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enter /the/ rest

@@ -366,6 +366,14 @@ Glossary
:term:`WSGI` components together declaratively within an ``.ini``
file. It was developed by Ian Bicking.

plaster
`plaster <http://docs.pylonsproject.org/projects/plaster/en/latest/>` is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add trailing _

Alternative Configuration File Formats
--------------------------------------

It is possible to use different file formats with :app:`Pyramid` if you do not like :term:`PasteDeploy`. Under the hood all command-line scripts such as ``pserve`` and ``pshell`` pass the ``config_uri`` (e.g. ``development.ini``, ``production.ini``, etc) to the :term:`plaster` library which performs a lookup for an appropriate parser. For ``.ini`` files it uses PasteDeploy but you can register your own configuration formats that plaster will find instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(for example, development.ini or production.ini)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with e.g.? It's not wrong and from a quick grep of our sources it's used in like 266 spots already.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Many style guides use plain English over the Latin abbreviation—"for example" is preferred over "e.g.", "in other words" and "that is" over "i.e.", and "and so on" over "etc.". Reasons include:
    • the intention is clearer
    • avoids misuse of the abbreviations, usually due to the lack of knowledge of Latin
    • has a less erudite tone
    • easier to translate
  2. The use of "e.g." with "etc." in the same phrase is redundant.
  3. If "e.g." or "i.e." must be used, then they are followed normally by a comma in American english, and confined within parentheses.

Eventually I would like to replace Latin abbreviations with common American english in the narrative documentation so that it reads smoother.

@mmerickel mmerickel dismissed stevepiercy’s stale review April 3, 2017 04:36

I don't intend to change the english words used at this time.

@mmerickel mmerickel merged commit e78aa24 into Pylons:master Apr 30, 2017
@digitalresistor
Copy link
Member

I forgot to comment on this, thanks Github UI. 👍 glad to see this happen :D.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants