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

emacs-packages: Drop deprecated package sets #66301

Merged
merged 55 commits into from Aug 30, 2019

Conversation

adisbladis
Copy link
Member

@adisbladis adisbladis commented Aug 7, 2019

Motivation for this change

These have been deprecated for a long time and I think we are doing our users a disservice by still keeping them around.

I tried porting over all packages that were not in any generated sets but some of these are broken.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @mdorman @thoughtpolice @peti @samuelrivas @etu

Note that I've left all derivations on purpose. I will make a follow up PR taking care of those once this one is merged.

@adisbladis
Copy link
Member Author

I think the release note could be better, any suggestions?

@oxij
Copy link
Member

oxij commented Aug 8, 2019 via email

@adisbladis
Copy link
Member Author

adisbladis commented Aug 8, 2019

@oxij I made a note of that Note that I've left all derivations on purpose. I will make a follow up PR taking care of those once this one is merged.

I see this as an excellent opportunity to clean up years of ad-hoc hacks and derivations.
While I'm a fan of aliases for backwards compat the current names we have are not matching well to the emacs ecosystem and I would rather drop them once and for all than having to keep aliases around for an eternity.
Personally I think the release notes as the correct place for this (and the release notes contains a warning about attribute names changing).

scalaMode for example becomes scala-mode. I've taken care to ensure that all packages that were previously available in emacsPackages are still available, just not under the same attribute names.

I'm open to keeping aliases for packages if people are vehemently for it but I really don't want to.

@oxij
Copy link
Member

oxij commented Aug 9, 2019 via email

@bendlas
Copy link
Contributor

bendlas commented Aug 12, 2019

currently running my system on this PR .. everything seems fine, so far. I have been on emacsPackagesNg for some time though ..

@adisbladis adisbladis force-pushed the emacspackages-deprecated-drop branch from a1e71e8 to ad76899 Compare August 18, 2019 16:07
@adisbladis
Copy link
Member Author

Well, leaving garbage in the tree until some later PR seems counterproductive to me.

OK fair enough, I've dropped all old packages in this PR.

I'm still on the fence about aliases, I don't think that the bisect use case is a good enough reason.
I'd like others to chime in with an opinion on this too.

I aim to merge this before the 19.09 release.

@oxij
Copy link
Member

oxij commented Aug 18, 2019 via email

@adisbladis
Copy link
Member Author

adisbladis commented Aug 28, 2019

cc @ttuegel @bendlas @matthewbauer

What do you think about the alias situation?

@bendlas
Copy link
Contributor

bendlas commented Aug 30, 2019

I'm a believer in the "don't break api" principle, so if somebody is willing to comb through the package sets, I'm for leaving the aliases in place, along with a deprecation warning.

@adisbladis adisbladis force-pushed the emacspackages-deprecated-drop branch from b0f7eb4 to aa9f330 Compare August 30, 2019 15:44
I want to add that I'm personally not a fan of this and I think we
should eventually drop these aliases to closely match the eco-system
rather than relying on some ad-hoc names created years ago.
@adisbladis adisbladis force-pushed the emacspackages-deprecated-drop branch from aa9f330 to b3d6dd5 Compare August 30, 2019 15:52
@adisbladis
Copy link
Member Author

I have added the requested aliases, even though I'm personally not a fan.

@adisbladis adisbladis merged commit f7b5be8 into NixOS:master Aug 30, 2019
@samuelrivas
Copy link
Contributor

Not sure if someone is still listening here but just in case:

It seems that the emacsPackages.colorThemeSolarized package marked as broken has an already fixed version in emacsPackages.solarized-theme. Should we just point the former to the latter for backwards compatibility (perhaps with some deprecation notice) or is there any value on keeping the broken one?

@samuelrivas
Copy link
Contributor

Although they are actually different packages, so that might not be a good idea after all

@samuelrivas
Copy link
Contributor

I just went ahead and fixed colorThemeSoloarized: #68768

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants