nfarina / homebridge-sonos

Sonos plugin for homebridge: https://github.com/nfarina/homebridge
155 stars 52 forks source link

Enable control of groups, information caching, add test environment for ease of development #77

Closed JohnathonMohr closed 4 years ago

JohnathonMohr commented 4 years ago

Note: the diff for index.js looks larger than it really is: a lot of it is indentation updates to make it easier to read. If you enable the "ignore whitespace" setting when reviewing, my updates will be clearer.

nfarina commented 4 years ago

Thanks for this contribution! My Sonos setup at home is very basic so I haven't had the need to really change this plugin in a long time.

Could you make these changes in a forked repo of your own that I can link to from the README here?

JohnathonMohr commented 4 years ago

I could keep a forked repo, but wouldn't that make it more complicated for others to get these features and fixes? There are a few users who have issues open that would be fixed with these updates, vs. asking them to start using a new fork (and likely a different npm package, if I can get one published - though I haven't actually published one before, so I don't know what the process is).

Is there a specific concern you have with the changes that I can address? I can put the extra functionality behind configuration params if you're more comfortable with that, or revert a lot of whitespace changes to reduce the diff size.

nfarina commented 4 years ago

Well I'm more worried about inadvertently disrupting any existing users of the plugin, then not having the bandwidth to deal with the support issues from that. I'd be happy to give you control of the repo if you'd like to manage that yourself though!

JohnathonMohr commented 4 years ago

I see. That's definitely understandable, and I appreciate your concern.

While I don't know how much time I can commit to managing issues broadly, I am willing to monitor and address any issues that others have as a result of my changes. I think it's fair I take responsibility for that. (Maybe that's what you meant, I'm just not 100% sure).

I think I'd enjoy trying to tackle other issues I see too if I do have the time though, so I would keep an eye on things anyway.

If that is what you end up deciding, I greatly appreciate it!

nfarina commented 4 years ago

Yes that's what I meant - and sounds good! I'll make you a contributor and you are welcome to merge this yourself whenever you like :)

JohnathonMohr commented 4 years ago

Thanks Nick! I'll go ahead and merge it now.

I'm assuming you'll take care of npm publishing...

nfarina commented 4 years ago

Ah forgot about npm! I can add you there as well if you like (just need your username); but I'll go ahead and publish now regardless.

JohnathonMohr commented 4 years ago

Great, thanks! My username is jmohr.