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

fix(colorscheme)!: rename colorshemes to guidelines #243

Merged
merged 1 commit into from
May 28, 2023
Merged

fix(colorscheme)!: rename colorshemes to guidelines #243

merged 1 commit into from
May 28, 2023

Conversation

TheSast
Copy link
Contributor

@TheSast TheSast commented May 28, 2023

Suggested in #242 (review)


Unrelated:
Shouldn't: README.md and CONTRIBUTING.md be in .github 🤔

@mehalter
Copy link
Member

They can be in either location for GitHub. In this case we put them front and center so people see them 😅

Copy link
Member

@mehalter mehalter left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@mehalter mehalter merged commit b860bd9 into AstroNvim:main May 28, 2023
@TheSast TheSast deleted the col-r-ename branch May 28, 2023 16:07
@nurseiit
Copy link

This change should probably be mentioned somewhere in the README. If I didn't run into this PR I wouldn't know I had to change "astrocommunity.colorscheme.tokyonight" to "astrocommunity.colorscheme.tokyonight-nvim" 🤦

@TheSast
Copy link
Contributor Author

TheSast commented May 30, 2023

@mehalter if these do get added to a README or a release note, we should probably fix all the plugins not following the convention, not just the colorschemes.

Also, do you think the version number should be bumped up after everything is renamed?

@mehalter
Copy link
Member

@TheSast this was decided it wasn't a breaking change genuinely based on the fact that this should have never been like this in the first place. If people want me to change this to a breaking change we can definitely do that

@mehalter
Copy link
Member

@TheSast I do think all plugins get fixed. If you know of more please open a PR to fix them :)

@mehalter
Copy link
Member

@TheSast i updated the commit message to show breaking

@benjaminmordaunt
Copy link

Awesome user experience! Break everyone's themes from community overnight!

@mehalter
Copy link
Member

mehalter commented Jun 2, 2023

Thanks @benjaminmordaunt ! ❤️

@luxus
Copy link
Member

luxus commented Jun 3, 2023

Awesome user experience! Break everyone's themes from community overnight!

I'm not a big fan of the big fan of the change, but nobody thought that astrocommunity gets so big..
the main issue is that keeping the foldername useful (name of the colorscheme) will have conflicts with themes with the same name etc.

@mehalter
Copy link
Member

mehalter commented Jun 3, 2023

Yeah I agree, also for people who rely on stability can track stable releases with semantic versioning to avoid random breaking changes on updates. Maybe we should just do a full refactor out to remove categories and go to a user/repo structure to completely avoid this issue in the future

@luxus
Copy link
Member

luxus commented Jun 3, 2023

not sure it makes sense.. there are some plugins that are actually 2 and maybe not a pack.. and packs are out of that anyways..
and the folders now help to structure the imports in your community.lua
i guess if you would dogfooding you would understand :D

@mehalter
Copy link
Member

mehalter commented Jun 3, 2023

Yeah that's true, good ol' Chesterton's fence at play. It's a shame that there are so many conflicts, but the current approach is pretty good/decent. We just have to make sure it is followed to avoid changing our minds down the line like how we got to this point. Definitely tough problem to solve

@TheSast TheSast changed the title fix(colorscheme): rename colorshemes to guidelines fix(colorscheme)!: rename colorshemes to guidelines Jul 12, 2023
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.

5 participants