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.

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

Allow default value for CoverArtCache path #2237

Merged

Conversation

pabera
Copy link
Collaborator

@pabera pabera commented Feb 4, 2024

No description provided.

@pabera pabera added the future3 Relates to future3 development label Feb 4, 2024
@pabera pabera added this to the v3.5 milestone Feb 4, 2024
@pabera pabera self-assigned this Feb 4, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 7776360839

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 38.2%

Totals Coverage Status
Change from base Build 7776323649: 0.0%
Covered Lines: 416
Relevant Lines: 1089

💛 - Coveralls

def __init__(self, cache_folder_path):
self.cache_folder_path = cache_folder_path
def __init__(self):
coverart_cache_path = cfg.setndefault('webapp', 'coverart_cache_path', value='../../src/webapp/build/cover-cache')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering is there a use case where a user needs that path configurable?

Copy link
Collaborator

@Groovylein Groovylein Feb 4, 2024

Choose a reason for hiding this comment

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

The question applies on many vars in the yaml.

I assume we can think of the yaml as a global var handler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm wondering is there a use case where a user needs that path configurable?

It's not required to be configurable, especially now that it is handled in its own class. Yet, it's a best practice at the moment. We might want to review the jukebox.yaml and check, if this could be done differently!

@pabera pabera merged commit 20389e3 into MiczFlor:future3/develop Feb 4, 2024
7 checks passed
@pabera pabera deleted the future3/fix-coverart-default branch February 4, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future3 Relates to future3 development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants