search

droidian / oFono2MM

Python daemom implementing ModemManager D-Bus API and using oFono to manage the modems.
BSD 3-Clause "New" or "Revised" License
8 stars 8 forks source link

Refactor code #2

Closed HenriDellal closed 3 years ago

HenriDellal commented 3 years ago

Needs review and testing

g7 commented 3 years ago

I have yet to test this, but generally looks ok to me. Does it work on your device?

Plus, this is my personal opinion - and you should wait for feedback from @erikinkinen - but backslashes are very un-pythonic and should not be abused. Please use implied line continuation when possible. Python's own PEP8 style guide might be a good inspiration.

I would also like for commits to be more descriptive, "Refactoring" is not really useful when looking through the repository history. "Styling and line-length changes" looks better for this specific case, in my opinion.

Thanks.

HenriDellal commented 3 years ago

I have yet to test this, but generally looks ok to me. Does it work on your device?

I don't have any device to test this on.

Plus, this is my personal opinion - and you should wait for feedback from @erikinkinen - but backslashes are very un-pythonic and should not be abused. Please use implied line continuation when possible.

I'll make changes as requested.

I would also like for commits to be more descriptive

Will do that as well.

erikinkinen commented 3 years ago

Looks good to me. I think this could be merged after squashing the refactoring commits.

erikinkinen commented 3 years ago

Closing as there is no activity.