GadgetReactor / pyHS100

Python Library to control TPLink Switch (HS100 / HS110)
Other
409 stars 128 forks source link

Project Management #40

Closed mweinelt closed 7 years ago

mweinelt commented 7 years ago

Hi,

here we are. Stuck for months now :(

I think it's advisable to fork as to not halt progress any further. There are a few pull requests in the pipeline, and the mess from the SmartBulb merge still has to be cleaned up.

I myself do not own a HS100, my parents do though , so testing is difficult for me.

Maybe @rytilahti or @kirichkov may want to chime in?

kirichkov commented 7 years ago

I fully agree with you! I'm fairly new to python and lack the programming experience and the knowledge to manage such a project. As a matter of fact this is the first piece of python code I've contributed to. For this reason I think @rytilahti is a much better choice to take the lead on this. I do have an HS110 and I'll definitely provide support for testing with the hardware and to the best of my abilities contribute code and advise.

GadgetReactor commented 7 years ago

please do!

or can I grant you full access to this repo or transfer?

sincere apologies. new work commitments and being a dad has taken its toll on my personal life!

mweinelt commented 7 years ago

We can work with this repository if we have more people who are able to review and merge.

kirichkov commented 7 years ago

Having access should be sufficient. I am hoping we can get someone who has both python experience and access to at least some of the hardware to lead this effort.

rytilahti commented 7 years ago

Hi everyone, I'd agree that shared maintainership on this repo (so no forking, that'll just create mess) would be the best way to go. I can keep making the releases & help with other things as before.

Thanks a lot @GadgetReactor for your work and congrats on the offspring!

kirichkov commented 7 years ago

Congrats on the offspring and thanks for your work from my end too. :-)

If @GadgetReactor wants we can indeed transfer the ownership - keep in mind he might prefer not to be getting notifications for a repo he can't pay attention to. I'd say it's up to him whether to transfer or give write/admin access.

I'd also wouldn't refuse to get write access in the case when @rytilahti is on a holiday or too busy with other projects in order to avoid stalling code merges.

rytilahti commented 7 years ago

Github allows collaborators (repository settings -> collaborators tab) which can be used to give access: https://help.github.com/articles/permission-levels-for-a-user-account-repository/ . The notifications can also be turned off for repositories, I think that would be the easiest way without moving things to new repository etc.

Although as the library is starting to gain support for other devices besides HS1(1|0)0, it could be discussed if there's a need for a rename some point in the future. Unfortunately I'm just in possession of one HS110 plug, so can't really comment much about other functionalities.

kirichkov commented 7 years ago

I've suggested that indeed a name change is needed to reflect the supported devices e.g. pyTPLSmartHome or something along those lines. I'm in the same boat hardware-wise. Only one HS110 and I'm not going to get any other plug as they are not stable in locations where voltage is unstable.

mweinelt commented 7 years ago

KISS: pytplink, pytpl

(tpl is the key-combo you have to type to enter their uboot on aps/routers)

GadgetReactor commented 7 years ago

pyTPLSmartHome 👍 or pyTPLKasa? pytplink might be too generic since it only works with the HS*** series devices and their other smart home products, and not their full range of stuff like routers, switches ..

GadgetReactor commented 7 years ago

I have added you three as collaborators - you guys have really been active and contributed lots. I think push access and full rights are up but let me know if more needs to be done.

Moving ahead, since there are a number of folks using this on Home Assistant, stability in releases should be top most priority. I kind of screwed this up midway, so hope we can fix this soon :)

kirichkov commented 7 years ago

I accepted the collaborator status already. Re: the name - pyTPLKasa also makes a lot of sense!

rytilahti commented 7 years ago

Thanks for granting the access @GadgetReactor, I already did some merging and issue clean-ups, a new release will follow later this weekend after getting that last PR properly merged. It'd be great to push a new version (and in case someone has the bulb & can make a homeassistant component for it) for the next homeassistasnt release.

About the naming, CamelCasing is not very pythonic, there has been already a discussion about renaming the module itself to lowercase to fix this :) Anyway, this is nothing we have to be hurried for now. I was thinking something in line with pykasa, python-kasa, python-tplink-smarthome, ..

mweinelt commented 7 years ago

@rytilahti I do like your naming ideas

GadgetReactor commented 7 years ago

thanks @rytilahti Ok let's stick to lowercase :)

rytilahti commented 7 years ago

Btw, about management and working on the master branch, I think the best way is to still use pull requests instead of directly commiting to the master. This way it's far more easier to do code review and avoids including code in which has not been approved by other commiters, I just created a couple of PRs #41 for the osx socket problem and #42 for a better testing tool.

Looks like @mweinelt approved the first one of them, great! I also recommend using git-extras (and it's awesome git-pr) to simplify working with PRs: https://github.com/tj/git-extras/blob/master/Commands.md#git-pr .

edit: one more thing to note, it's best to do merging with squash & merge, this will collapse the multiple commits into one for the history. Using PRs will also allow fixing the issues raised by hound and pushing amended commits without disrupting the master branch (that's what I just did for #42).

kirichkov commented 7 years ago

Yeah I agree. It's easier to track what was changed when there's a pull request especially if merged from a branch with a descriptive name

rytilahti commented 7 years ago

I think this is being dealt for now, thanks for raising the issue and thanks to @GadgetReactor for helping to fix it!