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

Issue #2656 #2853

Merged
merged 5 commits into from Dec 14, 2016

Conversation

Projects
None yet
4 participants
@mfrlin
Copy link
Contributor

commented Dec 7, 2016

Per #2851 I've fixed the files and submitting another pull request.

mfrlin added some commits Dec 6, 2016

@mmerickel

This comment has been minimized.

Copy link
Member

commented Dec 7, 2016

Would you please update the listen directives to support IPv6 interfaces as well, which is the major reason for moving to listen? For example, listen = *:6543 in production, and listen = 127.0.0.1:6543 [::1]:6543 for development. I think that is correct but need you to verify.

@bertjwregeer

This comment has been minimized.

Copy link
Member

commented Dec 7, 2016

Thanks! With the changes that @mmerickel suggested I think this will be good to go!

@mmerickel mmerickel added needs-work and removed do-not-merge labels Dec 8, 2016

mfrlin added some commits Dec 9, 2016

@mfrlin

This comment has been minimized.

Copy link
Contributor Author

commented Dec 9, 2016

I think it should be ok now. I've changed docs to shortly explain what *:6543 and [::] mean.

@stevepiercy
Copy link
Member

left a comment

Minor grammar changes.

@@ -316,12 +317,13 @@ the case, if you use a browser running on the same system as Pyramid, it will
be able to access the application via ``http://127.0.0.1:6543/`` as well as via
``http://192.168.1.50:6543/``. However, *other people* on other computers on
the same network will also be able to visit your Pyramid application in their
browser by visiting ``http://192.168.1.50:6543/``.
browser by visiting ``http://192.168.1.50:6543/``. Same holds true if you use

This comment has been minimized.

Copy link
@stevepiercy

stevepiercy Dec 9, 2016

Member

Minor grammar fix.

The same holds

@@ -316,12 +317,13 @@ the case, if you use a browser running on the same system as Pyramid, it will
be able to access the application via ``http://127.0.0.1:6543/`` as well as via
``http://192.168.1.50:6543/``. However, *other people* on other computers on
the same network will also be able to visit your Pyramid application in their
browser by visiting ``http://192.168.1.50:6543/``.
browser by visiting ``http://192.168.1.50:6543/``. Same holds true if you use
ipv6. ``[::]`` means the same as ``0.0.0.0`` but for ipv6 protocol.

This comment has been minimized.

Copy link
@stevepiercy

stevepiercy Dec 9, 2016

Member

More minor grammar fixes.

IPv6. [::] means the same as 0.0.0.0, but for IPv6.

127.0.0.1``), on port number 6543 (``port = 6543``). The server code itself
egg:waitress#main``), and it will listen on all interfaces (``listen =
127.0.0.1:6543 [::1]:6543``, means that it will listen on ipv4 and ipv6),
on port number 6543. The server code itself

This comment has been minimized.

Copy link
@stevepiercy

stevepiercy Dec 9, 2016

Member

Minor grammar fixes. Also you no longer need to hard wrap paragraphs.

and it will listen on all interfaces on port 6543 for both IPv4 and IPv6 (listen = 127.0.0.1:6543 [::1]:6543).

@stevepiercy
Copy link
Member

left a comment

Thank you!

@mmerickel

This comment has been minimized.

Copy link
Member

commented Dec 9, 2016

This is great. I'm leaving this open for now as a reminder, but feel free to help if you want it merged quicker!

  1. Update the cookiecutters as well.
  2. Update the changelog with the PR url. Also add note about it now working on IPv6.
  3. Don't merge until we fix the docs build errors on master then test this locally to make sure it still merges/builds.
@stevepiercy

This comment has been minimized.

Copy link
Member

commented Dec 10, 2016

Sphinx pinned to docutils 0.11 in sphinx-doc/sphinx#3217 to resolve sphinx-doc/sphinx#3212

@mmerickel mmerickel merged commit 3c5731a into Pylons:master Dec 14, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

mmerickel added a commit that referenced this pull request Dec 14, 2016

@mmerickel

This comment has been minimized.

Copy link
Member

commented Dec 14, 2016

I went ahead and updated the cookiecutters and triggered a new build on travis which passed. Thank you again for your work!

@mfrlin

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2016

Sorry, was kinda slammed this week and couldn't update the cookiecutters. It was my pleasure to contribute something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.