Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks for the submission. I think that the focus on Japanese readings gives this a useful distinction, particularly preserving supplied romaji and handling word boundaries across karaoke segments. (Yes, the goal of a centralised store is also to try to centralize efforts and reduce the number of forks to maximize collaboration) Before merging, there's an unload issue in both advertised releases, 7.2.0 and 7.2.1. hideUnwantedButtons sets inline display: none on Spotify's native fullscreen/PiP controls, but the registered cleanup callbacks never restore those styles. Disabling Sakura from Store unloads the module without restarting Spotify, so those controls can remain hidden. Could you publish a release that restores the previous styles and disconnects any pending observers through ctx.defer, then update this PR's artifact URL and checksum? If you're feeling adventurous, you can also add a regression test and verify that enabling, disabling, re-enabling, and uninstalling leave Spotify's native controls working without a restart. It would also help to include a short comparison with Lyrics Plus and a concrete Japanese lyric example showing the difference. Lyrics Plus already supports romaji and karaoke, so the listing should explain Sakura's specific reading improvements and distinguish them from the inherited Spicy Lyrics interface. CC: @rxri @veryboringhwl |
|
I don't think supporting forks in the module store is a good thing. |
|
@rxri I have had the same thought here, to be honest. And that's why I've specifically included that last paragraph. For these cases, I'd say that the best way forward is either to collaborate and continue on the original repo, or, if help is needed in the governance, we can move the repos inside the modules repo. HOWEVER, I do think that this module in particular already conflicts with an existing module, so my request would be to contribute back to the spicetify module, if it makes sense. (I have not deeply tested this module so I do not know what are the exact features it has over the already spicetify-owned module) |
|
spicetify org is definitely not a place for modules. modules should be in their own creator's/org's repos. If someone wants to add new functionality or expand it, they should do a pull request to the original module |
|
@rxri interesting. I have a bit of a different view. Happy to discuss it with others. I'd like to see the modules repo as a collaboration hub with a good CODEOWNERS that splits responsibility on the long term. But I do share your view that forks of things don't really belong in the marketplace, and ideally we should simply have customizable modules. Such as the debate of lyrics plus vs sakura lyrics. @theRealPadster as someone who has worked a lot towards marketplace, what do you think? |
It should only be for official spicetify modules. v3 already pins the hash so I genuinely don't see any sense in keeping them in one repo |
|
I think we’re mixing a few separate questions here: what belongs in the Store, where its code lives, and who maintains it. I agree that authors should be able to keep modules in their own repos. But hash pinning doesn’t remove the benefits of shared maintenance. It verifies the downloaded artifact; it doesn’t help coordinate compatibility fixes or keep a module maintained when its author steps away. That’s where I see value in the modules repo as an optional collaboration hub, with clear ownership and expectations. I’d also soften my earlier position on forks. Contributing upstream should be encouraged, but I don’t think being a fork should automatically disqualify something from the Store. The questions should be whether it offers a meaningful difference, is maintained, credits its origins clearly, and meets our quality requirements. For Sakura specifically, I think my initial requests still make sense: fix the unload issue and show concrete examples of what its Japanese readings improve. Also, contributing to Spicy Lyrics, its actual upstream, is different from porting those features into Lyrics Plus. We should probably establish the differences before deciding consolidation is the right outcome. My preference would be a curated Store, independent repos by default, and optional shared stewardship for maintainers who want it. We can encourage collaboration without requiring every alternative to become another setting in one module. It would probably help to move the broader governance discussion into its own thread and give this submission a clear acceptance path. |
I would agree with everything here. I think it's important that authors are able to have their modules in their own repo. I like how current Spicetify has an official set, and then anything else is controlled by the community. It would be a lot more work if it all needed to go through the Spicetify repos. Forks can be annoying if they are too similar or offer little value/differentiation from the original work. But just because something is based on something else doesn't automatically mean it shouldn't be allowed. If the original author has a difference of opinion, or the change doesn't fit with what the original author is trying to do, that would make sense to create a fork (assuming it's not something small/trivial). |
|
Thanks, @theRealPadster, that matches what I had in mind. Shared maintenance would be optional, with explicit agreement on who takes responsibility. Independent community repos should remain the default. I also agree that the degree of differentiation matters more than whether something started as a fork. Let's move the general Store admission and maintenance policy into a separate discussion, so contributors have clear expectations for future submissions. @JosEffigy, sorry for taking over your PR with the broader governance discussion. For this submission, I'd keep the next steps concrete: resolve the unload issue and demonstrate the Japanese-reading improvements so we can assess whether a separate listing makes sense. |
Adds sakura-lyrics@7.2.0 as a new Store entry.
Sakura Lyrics is an unofficial fork of Spicy Lyrics, created by Spikerko. Spikerko is credited first in the package and listing, and the README explains the inherited renderer, design and feature code. JosEffigy maintains this fork. It is not an official or endorsed Spikerko release.
Disclosure: this is a vibe-coded project developed with substantial AI assistance. The fork focuses on Japanese romaji (phrase readings, small tsu, spacing and karaoke boundaries) and reducing unnecessary rendering work. Live CPU/GPU savings have not been measured; dictionary errors and approximate within-word timing remain possible.
The package keeps AGPL-3.0-only and upstream copyright notices. The preview is inherited upstream artwork, explicitly credited in the README. Source: https://github.com/JosEffigy/spicy-lyrics-v3
Validation: 23 regression tests, build and package checks pass; the registry's validate-submission script passes against the published asset. This PR changes only vault/sakura-lyrics.json. Users should not enable the old spicy-lyrics package alongside this one.