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

Demo integrating pycket into cyclone #3

Merged
merged 2 commits into from Aug 30, 2012

Conversation

ytjohn
Copy link
Contributor

@ytjohn ytjohn commented Aug 30, 2012

Cyclone Redis Demo

See github/firoix/cyclone for the latest source.

About

This demonstrates integrating pycket with cyclone, using a redis backend (but easily switched to using memcached).

Cyclone is a clone of facebook's Tornado, on top of Twisted.

Although cyclone and tornado are very similar, cyclone leverages all
enterprise class features of Twisted, and more. It is extremely stable, and
ready for production.

@travisbot
Copy link

This pull request passes (merged c4b6fd1 into 170c925).


def get(self):
self.write('<form method="post">Enter your username: <input name="username" type="text"><button type="submit" class="btn">Login</button></form>')

Copy link
Owner

Choose a reason for hiding this comment

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

Could you remove this extra blank line, so that we can keep up on PEP8?

@diogobaeder
Copy link
Owner

Nice job, @ytjohn ! :-)

Just two points I'd like to make:

  1. I'm not comfortable with a piece of code with a different license other than the package's original one - that is, Apache License instead of BSD 2-clause -. Do you think you can remove this part and leave it as implicit BSD 2-clause, as the rest of the package?
  2. There's no test for this demo, and currently pycket has 100% coverage, which is a status I'd like to keep. Can you make some tests for it before I merge this back? There's no need to test the ioloop, as the cost/benefit for this does not pay the effort, but I think the rest of the demo can be tested appropriately. I know that the demo resides outside of the pycket package, but it would be nice to publish a demo that has a proof that it actually works. :-)

Cheers!

@ytjohn
Copy link
Contributor Author

ytjohn commented Aug 30, 2012

@diogobaeder , I updated the demo to PEP-8 compliance and made the license implicit. I am going to look into creating some tests.

@travisbot
Copy link

This pull request passes (merged 4cbfb3f into 170c925).

diogobaeder added a commit that referenced this pull request Aug 30, 2012
Demo integrating pycket into cyclone
@diogobaeder diogobaeder merged commit 03246e1 into diogobaeder:master Aug 30, 2012
@diogobaeder
Copy link
Owner

I've merged your request, please let me know when you have some tests for it.

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.

None yet

3 participants