Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Remove Python 2 compatibility code #6

Closed
wants to merge 6 commits into from
Closed

Remove Python 2 compatibility code #6

wants to merge 6 commits into from

Conversation

berkerpeksag
Copy link
Contributor

Closes #5

This pull request also includes three cleanups in separate commits.

@facebook-github-bot
Copy link

By analyzing the blame information on this pull request, we identified @pallotron to be a potential reviewer.

@pallotron
Copy link
Contributor

good stuff, I have been working on some of this already, you should see a commit landing to master shortly, please rebase your repo and resolve any merge conflict, if any.

@pallotron
Copy link
Contributor

made some changes in a4780e8, feel free to rebase/resolve conflicts now :)

@pallotron
Copy link
Contributor

can you also change the Example section in the README.md file? :)

@berkerpeksag
Copy link
Contributor Author

Will do in a moment, thanks!

By the way, using super(self.__class__, self) over super(ClassName, self) (or simply super()) can cause subtle bugs in subclasses since the former will try to invoke subclass' method instead of parent's method. Here is a good example to demonstrate the problem: https://gist.github.com/ulope/1935894

Do you want me keep them as in a4780e8 or change them to super(ClassName, self) or super()?

@pallotron
Copy link
Contributor

you can change them as long as make test passes.

@pallotron
Copy link
Contributor

any update?

@berkerpeksag
Merge branch 'master' of https://github.com/facebook/fbtftp into rm-p…
3977dec 
…ython2

Conflicts:
	examples/server.py
	tests/base_handler_test.py
	tests/base_server_test.py
	tests/integration_test.py
	tests/malformed_request_test.py
@berkerpeksag
Update example to use super()
b9f0f5b
@facebook-github-bot
Copy link

@berkerpeksag updated the pull request.

@pallotron
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot
Copy link

Thanks for importing. If you are an FB employee go to Phabricator to review.

@ghost ghost closed this in f47bcf7 Apr 8, 2016
@berkerpeksag berkerpeksag deleted the rm-python2 branch April 8, 2016 15:44
@berkerpeksag
Copy link
Contributor Author

Thanks for merging this so quickly. I'm glad Facebook is using Python 3 on production :)

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed GH Review: accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@berkerpeksag @facebook-github-bot @pallotron