Skip to content
This repository has been archived by the owner on Oct 1, 2020. It is now read-only.

Support to python3 #190

Merged
merged 19 commits into from Jun 4, 2015
Merged

Support to python3 #190

merged 19 commits into from Jun 4, 2015

Conversation

alexandre
Copy link

All tests are passing, but I'm referencing some own repositories while I not receive a feedback about my others PR.

I've created a repository for the twill with support to python3 (2to3) and I would like to pass this repository to the quokkaproject profile, if you want...of course.

@ghost ghost assigned rochacbruno May 3, 2015
@ghost
Copy link

ghost commented May 3, 2015

👍

1 similar comment
@avelino
Copy link

avelino commented May 3, 2015

+1

@rochacbruno
Copy link
Collaborator

👍 very nice! @alexandre I will take a closer look and merge soon!

@alexandre
Copy link
Author

@rochacbruno ,

I'm updating the PR with the new feature (RSS) and keeping the python3 support. There are some PEP8 warnings, I'll fix it too, ok?

@ellisonleao
Copy link

nice one!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.43%) to 60.64% when pulling 1153652 on alexandre:master into 4477014 on quokkaproject:master.

markupsafe
Pillow
pyyaml
speaklater
https://github.com/pythonhub/quokka-themes/tarball/master
https://github.com/alexandre/quokka-themes/tarball/master
Copy link
Collaborator

Choose a reason for hiding this comment

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

I merged your fork with quokkaproject/quokka-themes

@coveralls
Copy link

Coverage Status

Coverage increased (+0.43%) to 60.64% when pulling 8783a26 on alexandre:master into 4477014 on quokkaproject:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.43%) to 60.64% when pulling 8783a26 on alexandre:master into 4477014 on quokkaproject:master.

@@ -40,7 +40,8 @@ def inner(*args, **kwargs):
def _get(self):
def inner(*args, **kwargs):
values = only_matches(self, kwargs)
if len(values) > 1:
values = list(values)
if len(list(values)) > 1:
Copy link
Member

Choose a reason for hiding this comment

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

We need to perform this cast twice?
Why not just?

values = list(values)
if len(values) > 1:

Copy link
Author

Choose a reason for hiding this comment

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

We should refactor this piece...I didn't see that because I did keep my focus on python3 support.
I'll fix this flags. =]

Copy link
Member

Choose a reason for hiding this comment

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

Cool.
BTW is a great job, congratulations 👏 .

@rochacbruno
Copy link
Collaborator

@alexandre the twill author answered your email?

we need to merge this PR soon, before code changes a lot.

I want to include your changes, only need to know what to do about twill and decide about the unicode issue above.

@alexandre
Copy link
Author

@rochacbruno no email after that one. I think you can keep the repo on quokkaproject.

I stop [for a moment] my commits because personal problems (laptop broken). But I'll work on this today.

[ ]'s (hugs)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.42%) to 60.69% when pulling f553c75 on alexandre:master into 9bc994d on quokkaproject:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.42%) to 60.69% when pulling f553c75 on alexandre:master into 9bc994d on quokkaproject:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.42%) to 60.69% when pulling f553c75 on alexandre:master into 9bc994d on quokkaproject:master.

@alexandre
Copy link
Author

updated

forbidden = False
break # break in first occurence
forbidden = next(
(True for role in user.roles if role in user.roles), False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess there is a problem here:

the loop should iterate channel.roles

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will merge, and then try to think in a way to write a test for this feature.

@rochacbruno rochacbruno merged commit f553c75 into quokkaproject:master Jun 4, 2015
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.

None yet

7 participants