search

mtudor / HomeySqueezebox

Control your Logitech Squeezebox server and devices with your Athom Homey!
4 stars 10 forks source link

New version #1

Open Inversion-NL opened 7 years ago

Inversion-NL commented 7 years ago

Move server settings to device pairing for multiple server support Added next, previous, seek, random and shuffle cards Mobile card with on/off, play/pause, random and shuffle Set device online and offline in Homey

mtudor commented 4 years ago

Hi @Inversion-NL. When you're submitting pull requests can you please separate them into individual requests that address specific functionalities. Having them all in a single pull request makes it a really big job to review and delays acceptance of the pull request as a whole. For example, if your change to update to Homey 2.0 is just, say, 50 lines then I can review that quickly and get it merged for people to use. If those 50 lines are in a pull request with 500 other lines to add various other functionalities then I have to review all 550 lines before I can merge the change that upgrades the app to Homey 2.0.

If instead you have, for example, a PR which just updates the app for Homey 2. Then another that adds additional flow cards. Another that adds auto-discovery etc. It means that I can review a small subset of the code and get that merged to master more quickly. You can do it by creating branches in your local github repository and raising a pull request for each subset of functionality.

Does that make sense?

Inversion-NL commented 4 years ago

But all had to be changed to have it working again.

Op wo 18 sep. 2019 10:58 schreef Mark Tudor notifications@github.com:

Hi @Inversion-NL https://github.com/Inversion-NL. When you're submitting pull requests can you please separate them into individual requests that address specific functionalities. Having them all in a single pull request makes it a really big job to review and delays acceptance of the pull request as a whole. For example, if your change to update to Homey 2.0 is just, say, 50 lines then I can review that quickly and get it merged for people to use. If those 50 lines are in a pull request with 500 other lines to add various other functionalities then I have to review all 550 lines before I can merge the change that upgrades the app to Homey 2.0.

If instead you have, for example, a PR which just updates the app for Homey 2. Then another that adds additional flow cards. Another that adds auto-discovery etc. It means that I can review a small subset of the code and get that merged to master more quickly. You can do it by creating branches in your local github repository and raising a pull request for each subset of functionality.

Does that make sense?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mtudor/HomeySqueezebox/pull/1?email_source=notifications&email_token=ADZO66AGAC5RIO5M6YICTI3QKHUTBA5CNFSM4CQAUUNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD67LDPA#issuecomment-532591036, or mute the thread https://github.com/notifications/unsubscribe-auth/ADZO66DAMZCRDO2IPF2X2TTQKHUTBANCNFSM4CQAUUNA .