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

Rename shared library to match package name #63

Closed
wants to merge 1 commit into from
Closed

Rename shared library to match package name #63

wants to merge 1 commit into from

Conversation

tarsius
Copy link
Contributor

@tarsius tarsius commented Oct 28, 2016

A package should come with a library that matches its name.
The commentary of that library will then be used on Melpa as
the package description.

A package should come with a library that matches its name.
The commentary of that library will then be used on Melpa as
the package description.
@tarsius tarsius closed this Nov 30, 2016
@tarsius tarsius reopened this Nov 30, 2016
@nashamri
Copy link
Owner

@tarsius thanks for this, I've been really busy lately. So, sorry for talking so long for me to reply.
I'm just wondering if this change will cause any issues with either MELPA or spacemacs.

I haven't looked into it yet, but if you have any thoughts on whether this change will cause any issues, I'm all ears 😄

@tarsius
Copy link
Contributor Author

tarsius commented Nov 30, 2016

Actually I have learned that this change can be problematic after I have proposed it, see melpa/melpa#3668 (comment). I intend to work with Steve to come up with a suitable convention for such two-variant-themes, but haven't gotten around to that yet.

I recommend you don't merge this just yet, or even close it. Once I have figured things out I will update this pull request or open a new one.

@nashamri
Copy link
Owner

nashamri commented Nov 30, 2016

@tarsius oh brilliant, thanks. I guess being lazy can be beneficial sometimes 😄

@nashamri
Copy link
Owner

Actually we did discuss that issue a bit, me and Steve, in my original PR for spacemacs-theme to be added to MELPA:
melpa/melpa#2896

I do hate that the commentary section is not showing up in the theme MELPA page https://melpa.org/#/spacemacs-theme

@tarsius
Copy link
Contributor Author

tarsius commented Nov 30, 2016

Ah, thanks for that pointer! Well, I can't really think of a satisfactory solution, which is part of the reason I haven't started a conversation about this yet. (But if you have any ideas...)

@tarsius tarsius closed this Dec 1, 2017
@tarsius
Copy link
Contributor Author

tarsius commented Nov 20, 2022

I would like to revisit this.

At melpa/melpa#2896 you said.

I like the idea of getting rid of the -pkg-el file but to me I think renaming the package to spacemacs-themes gives the idea of having more than one theme. While in reality, it's only one theme with two variants, like solarized theme for example.

Technically two "variants of a theme" are still two themes, so I don't think this is misleading. Personally I do use "solarized light" and consider it a theme that is very different from "solarized dark". Most existing packages named *-themes do in fact provide two variants of a theme (usually a dark and a light one), and not a collection of themes whose only shared characteristics are that they are distributed in the same repository and have the same author.

But if you really want to make it 100% clear that these are variants, then you could also name the package spacemacs-theme-variants (and rename the library accordingly).

So I think I'll stick with the current approach.

Please reconsider. There are only a handful of packages remaining on Melpa whose names doesn't match the names of their "main libraries", and I would like to get to zero soon.

I do hate that the commentary section is not showing up in the theme MELPA page https://melpa.org/#/spacemacs-theme

This is a consequence of the mismatch between the package and library name. The only way to fix it is to remove that mismatch.

@tarsius
Copy link
Contributor Author

tarsius commented Nov 20, 2022

This old pull-request just renames the library from spacemacs-common.el to spacemacs-theme.el (singular).

I recommend that you instead rename the library and package to spacemacs-themes, because otherwise load-theme would offer spacemacs-theme as theme that can be loaded and if the user chooses that, then they get an error.

@nashamri nashamri self-assigned this Nov 21, 2022
@nashamri
Copy link
Owner

Hello @tarsius 😃
Sure thing! let me take a look at this. Hopefully I'll have some free time this weekend.

@APIPLM
Copy link

APIPLM commented Dec 7, 2022

@nashamri . Thanks. The theme is much more like products level thing. As you release one of emacs, it has variants of them as your production options.So the name of the package might be different to the name of the module what we have on MELPA.

@tarsius
Copy link
Contributor Author

tarsius commented Mar 11, 2023

Friendly ping!

I have another question. The package is named spacemacs-theme. Does that mean that it was extracted from Spacemacs (1) so that it could be used without also using Spacemacs, or (2) does Spacemacs itself now get its default theme from here, i.e. was this theme removed from the Spacemacs repository. I assume it is the former, but I think this should be clarified in the README.

@nashamri
Copy link
Owner

Hey @tarsius and I'm sorry I haven't got the time to look at this issue and your #199, been really busy.

I like your PR and I think it makes more sense but one of the main things that I'm a little bit worried about is breaking speacemacs theme (in spacemacs distribution, not here). Because spacemacs pulls the theme from here and use it as one of the core libraries, as you can see here:
https://github.com/syl20bnr/spacemacs/tree/develop/core/libs
And I think the process is automated too. Once I push anything here, it is pulled in spacemacs distribution right away.

So yeah, you assumption is right. I guess this needs some coordination and changes in spacemacs too to prevent any breakages.

@tarsius
Copy link
Contributor Author

tarsius commented Mar 11, 2023

Ah, then we definitely have to coordindate with Spacemacs. Since you are busy, I am fine with further delaying this some more. Provided we can get it done eventually. 😁

@nashamri
Copy link
Owner

Hopefully we will get there... before the end of times 😄

P.S. Thank you @tarsius for all of the amazing work you are doing for emacs 👍

@tarsius
Copy link
Contributor Author

tarsius commented Mar 11, 2023

Thanks!

@tarsius
Copy link
Contributor Author

tarsius commented Apr 15, 2023

Renaming the package (as proposed in #199) takes some work. Since you do not have the time to do that, I would like to ask you to merge this instead. Doing that shouldn't take any coordination. Users already require either spacemacs-dark-theme or spacemacs-light-theme, which then requires spacemacs-common. Renaming that to spacemacs-theme should be an entirely internal matter. It would be great if you could do that, because then there would be zero packages on Melpa that do not provide a feature matching the package name.

@nashamri
Copy link
Owner

All righty, let's do this. Just pushed an update that does that (c98c156).

@tarsius
Copy link
Contributor Author

tarsius commented Apr 25, 2023

Thanks!

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.

3 participants