Skip to content

Conversation

tobimichael96
Copy link
Contributor

@tobimichael96 tobimichael96 commented Oct 6, 2019

Added option to use credential-only caching as described in issue #351

By default this uses credential-only caching which might be not the best approach, as the default value was hardcoded with "true".

Let me know if you want to change this.

Disclaimer: I'm pretty new to all of this, so just tell me if I did something stupid.

Edit: Fix #351

Copy link
Member

@mainrs mainrs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make things easier and not bump for a minor version change I would rather make the default value true.
Oh, and thanks for spending some time on Spotifyd :)

@mainrs mainrs added this to the v0.2.19 milestone Oct 7, 2019
@mainrs
Copy link
Member

mainrs commented Oct 7, 2019

For future reference, take a look at this. You can edit your first comment and github will still recognize it :)

I really need to get the issue and PR templates going. Would make it way easier for others to contribute :)

@tobimichael96
Copy link
Contributor Author

tobimichael96 commented Oct 7, 2019

For future reference, take a look at this. You can edit your first comment and github will still recognize it :)

I really need to get the issue and PR templates going. Would make it way easier for others to contribute :)

Thank you, I'll use this in the future!

Edit: Added the fix comment to the initial post, I hope it does work like this.

@tobimichael96
Copy link
Contributor Author

To make things easier and not bump for a minor version change I would rather make the default value true.
Oh, and thanks for spending some time on Spotifyd :)

Alright, I change the things as you said, I hope it's ok now.

No need to thank me, I'm using it on a daily basis, so I wanted to give something back (even if it's pretty small compared to what you do)!

@mainrs mainrs marked this pull request as ready for review October 7, 2019 08:47
@mainrs mainrs merged commit 81675fa into Spotifyd:master Oct 7, 2019
@tobimichael96
Copy link
Contributor Author

tobimichael96 commented Oct 7, 2019

Maybe we should add the option in the README like this:

# ~/.config/spotifyd/spotifyd.conf
[global]
volume_controller = alsa                                    # or alsa_linear, or softvol
# on_song_change_hook = command_to_run_on_playback_events
device_name = device_name_in_spotify_connect             # must not contain spaces
bitrate = 160                                            # or 96, or 320
cache_path = cache_directory
no_audio_cache = true                           # to use credential-only caching
volume_normalisation = true
normalisation_pregain = -10

@mainrs
Copy link
Member

mainrs commented Oct 7, 2019

Probably ye :) I'll do it quick. #358

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow credential-only caching
2 participants