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

chore: remove unused xtask themelint #10294

Merged

Conversation

AlexanderBrevig
Copy link
Contributor

@David-Else
Copy link
Contributor

I have never understood why the key maintainers don't like this feature. The current situation where sometimes the majority of themes don't actually work properly with the latest Helix release is bad, and here we have the possible solution (or at least help), but it has been rejected with no alternative offered. There is no official way to communicate to the authors that their theme needs maintenance, if this is not addressed many themes will just decay and the overall Helix user experience will suffer.

I was thinking the theme situation is pretty chaotic and would really benefit from a mass lint and possible pruning. Maybe there could be a link in the next release's release notes to a GitHub discussion poll where people vote to show what theme they are using, themes that nobody votes for and are failing the lint could be pruned away, and popular themes that are missing keys could have the issues addressed.

@the-mikedavis
Copy link
Member

Historically speaking, this was merged too fast - before we discussed the PR internally. It's extra maintenance burden on top of maintaining themes for something I doubt could actually prevent all or even most of the theme problems we see in issues/PRs. Plus it's hard to say what can be linted since theme choices are subjective.

I agree that theme maintenance is in a bad way but I'm highly skeptical that linting is the solution. We don't have concrete plans for it yet but we've planned to cut down on the number of themes in the repo for a while. I'd like to see us maintain only some constant number of big name themes (nord, catppucchin, tokyonight, gruvbox, github, ...) and drop the rest, especially personal or novel themes. Most themes are better off in their own standalone github repo so that the author can be notified when changes are needed, as you say. A lint in this repo just complicates adding features that introduce new theme keys. We'd be stuck from merging a PR that adds a theme key until all themes pass the lint.

@pascalkuthe pascalkuthe merged commit be8afe1 into helix-editor:master Apr 8, 2024
6 checks passed
@AlexanderBrevig AlexanderBrevig deleted the remove-unused-themelint branch April 8, 2024 17:46
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 20, 2024
* chore: remove unused xtask themelint

* chore(clippy): remove unused themes
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
* chore: remove unused xtask themelint

* chore(clippy): remove unused themes
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
* chore: remove unused xtask themelint

* chore(clippy): remove unused themes
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
* chore: remove unused xtask themelint

* chore(clippy): remove unused themes
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.

4 participants