-
Notifications
You must be signed in to change notification settings - Fork 461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce use_mpris configuration setting #740
Conversation
Looks like the linter error is not in the code I've touched. Running the same |
Well, I've tried to come-up with this new feature and now I'm dragged down by errors with the linter on the code I don't touched and that are not reproducible on my laptop. Looks like the master code-base is not clean from the linter PoV. Please help me finish-up this pull request! |
Hey @valir thanks so much for the PR on the issue i made! ill quickly check out the code and test it. as for the CI other PRs also have this issue and i dont think it should be adressed here, when we fix it you can rebase/merge dev into this branch and the lint errors should go away. Sorry for the inconvience |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good, and quickly testing it seems to work great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the value should also be merged by the configs, at https://github.com/Spotifyd/spotifyd/pull/740/files#diff-cba64c21ab992eaad29fce147a08f4560a4769bc14682b8a96081a5fd02dbecdR517
BUG:730 This defines the `use_mpris` configuration setting in the main configuration file. Set this to `false` in case you have a headless spotifyd instance, running in session lacking a dbus session.
After fetching upstream and rebasing on that master, I'm still getting |
Yea that is unfortunately currently an issue. But we can just merge it even if there are lint errors |
Looking forward for the merge as I'd like to see it landing on Archlinux Arm repos 😉 |
@valir maybe i missed it but did you adress this comment already? #740 (review) |
The link in there shows an overview of the diffs my branch brings-in. Looks like I don't get what should be done there. |
oh yeah sorry about that it should have gone to a specific line, the function config.rs:SharedConfigValues:merge_with merges the config from disc and from the command line. If you could add the new field so that it is also merged |
#730
This defines the
use_mpris
configuration setting in the mainconfiguration file. Set this to
false
in case you have a headlessspotifyd
instance, running in session lacking a dbus session.