Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add documentation to configuration fields that are not allowed under some circumstances #394

Open
mainrs opened this issue Oct 17, 2019 · 7 comments · May be fixed by #800
Open

add documentation to configuration fields that are not allowed under some circumstances #394

mainrs opened this issue Oct 17, 2019 · 7 comments · May be fixed by #800
Labels
enhancement A new feature that would improve Spotifyd good first issue An easy to implement issue, good for first time contributors or people new to open source help wanted Issues that need help since the assigned person has little to no knowledge about the topic pinned Apply to make stale bot ignore issue/PR.
Milestone
v0.4.0

Comments

@mainrs
Copy link
Member

mainrs commented Oct 17, 2019

For example: mixer, control and device only apply if the alsa_backend feature is enabled (I think, someone needs to check against this before starting to implement stuff). No idea how to do this in a clean way. Maybe a cfg_attr checking for the feature and only adding the fields to the SharedConfigValues struct if that is the case.

If the above method works, this should actually be a pretty easy enhancement :)
@mainrs mainrs added enhancement A new feature that would improve Spotifyd help wanted Issues that need help since the assigned person has little to no knowledge about the topic good first issue An easy to implement issue, good for first time contributors or people new to open source labels Oct 17, 2019
@mainrs mainrs added this to the v1.0.0 milestone Oct 20, 2019
@stale
Copy link

stale bot commented Jan 13, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix Issues that will not be fixed under any circumstances label Jan 13, 2021
@slondr slondr added the pinned Apply to make stale bot ignore issue/PR. label Jan 16, 2021
@stale stale bot removed the wontfix Issues that will not be fixed under any circumstances label Jan 16, 2021
@slondr slondr modified the milestones: v1.0.0, v0.4.0 Jan 16, 2021
@CPerezz
Copy link
Contributor

CPerezz commented Jan 30, 2021

Hi @sirwindfield . I wanted to help. But I'm not sure about the scope of the PR.

If the goal is to conditionally include fields (not only docs) to SharedConfigValues, it might be quite challenging, since I tried a bit and will affect other structs such as SpotifydConfig as well as changes in macro definitions.

Maybe I missunderstood the issue and the only intention is to have these fields hidden from the documentation when alsa-backend feature is not enabled. On which case, I think I can make a quick PR.

Let me know if I can help! 😃

@mainrs
Copy link
Member Author

mainrs commented Jan 30, 2021

I think the idea was to use cfg_attr(feature = "alsa_backend") for example on a field. That way, if the alsa backend is enabled, it would include the struct field, otherwise not. Since alsa-related code is only included if alsa is enabled, there shouldn't be any access to the fields anyway so it can be removed.

I am not sure if this is still a good idea though user-wise. It would force people to change their config, but I think it's actually good that way.

@Icelk
Copy link
Contributor

Icelk commented Feb 19, 2021

Hello @sirwindfield . It seems @CPerezz has not made any progress, but can I help and make a PR?

@CPerezz
Copy link
Contributor

CPerezz commented Feb 19, 2021

Hello @sirwindfield . It seems @CPerezz has not made any progress, but can I help and make a PR?

I have a PR pending for review. Forgot to link it... Let me do so

@CPerezz CPerezz linked a pull request Feb 19, 2021 that will close this issue
Open
@Icelk
Copy link
Contributor

Icelk commented Feb 19, 2021

Right, thanks. And great work! 👍

@MJWcodr
Copy link

MJWcodr commented Aug 23, 2023

is this still open?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement A new feature that would improve Spotifyd good first issue An easy to implement issue, good for first time contributors or people new to open source help wanted Issues that need help since the assigned person has little to no knowledge about the topic pinned Apply to make stale bot ignore issue/PR.
Projects
None yet
Milestone
v0.4.0
Development

Successfully merging a pull request may close this issue.

Refactor Config structs according to features CPerezz/spotifyd
5 participants
@mainrs @slondr @CPerezz @Icelk @MJWcodr