Skip to content

Python 2 -> 3 part 1 #1746

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

Closed
wants to merge 7 commits into from
Closed

Python 2 -> 3 part 1 #1746

wants to merge 7 commits into from

Conversation

iljah
Copy link

@iljah iljah commented Mar 23, 2021

Few changes to support python 3. Supercedes #1745. Works with python 2.7.15 in fedora 29.

@iljah iljah mentioned this pull request Mar 23, 2021
@iljah
Copy link
Author

iljah commented Mar 23, 2021

Not sure if I should remove check for python 3 in 7c66577 before everything works

iljah added a commit to iljah/PyBitmessage that referenced this pull request Mar 23, 2021
Otherwise I get this with python 3.8.7 when starting BM: TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType' (assuming changes in Bitmessage#1746)
@g1itch
Copy link
Collaborator

g1itch commented Mar 24, 2021

Not sure if I should remove check for python 3 in 7c66577 before everything works

You shouldn't. Again, please read #1712.

@g1itch
Copy link
Collaborator

g1itch commented Mar 24, 2021

Maybe you begin with some tests? Separate TestConfig from TestProcessConfig like I did here: d947e70 because the later wont work before complete port to python3 (it uses subprocess to call pybitmessage -t) and write more, e.g. 9808968.

It looks like in python3 you can reuse defaults from BMConfigDefaults and then getint(), getboolean() will act in the same way as safeGetInt() and safeGetBoolean() in current code.

Porting BMConfigParser() is the next big thing in python3 porting process, so I'd suggest to separate related changes in dedicated branch. You may also mock BMConfigParser() and write tests and changes for some other parts of code.

@g1itch
Copy link
Collaborator

g1itch commented Mar 24, 2021

Notice the required checks

This reverts commit 7c66577.
@iljah
Copy link
Author

iljah commented Mar 25, 2021

Maybe you begin with some tests?

Could work. I've started that in https://github.com/iljah/PyBitmessage/tree/py3-tests1 and once I get bit further I'll submit another pull request.

@iljah
Copy link
Author

iljah commented Mar 25, 2021

After #1749 I start hitting errors that are fixed by stuff here so I guess I'll wait until this is merged until continuing work on above test fixes

@g1itch g1itch closed this Aug 11, 2021
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.

2 participants