thorpelawrence / alexa-spotify-connect

Control Spotify Connect devices with Alexa
MIT License
97 stars 53 forks source link

Support device names #69

Closed NicoWeio closed 4 years ago

NicoWeio commented 4 years ago

Apart from translations, this should be ready for a merge. Hope you like it! :) closes #1

thorpelawrence commented 4 years ago

Thanks for this! Were you able to get the tests to pass? For some reason it looks like travis has not picked up this PR

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 317


Changes Missing Coverage Covered Lines Changed/Added Lines %
connect.js 24 28 85.71%
device-helper.js 7 11 63.64%
<!-- Total: 31 39 79.49% -->
Totals Coverage Status
Change from base Build 316: -2.2%
Covered Lines: 149
Relevant Lines: 182

💛 - Coveralls
NicoWeio commented 4 years ago

Maybe you've disabled CI for PRs? Anyway, my code actually wasn't that ready for a merge: I fixed two bugs, and fixed the testing. As I modified some strings, there are now missing translations in es, fr, it and pt.

thorpelawrence commented 4 years ago

Looks like travis is just being slow today, and it's passed now so thanks - as you say about the translation, I'm not sure what to do about that as they were contributed by other people, so maybe they will need to be fixed later