-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
vimPlugins.coc-prettier: Add override #72506
Conversation
since the 2 commits are necessary to perform the fix, do you mind squashing them both with the commit message:
|
@jonringer done! |
@GrahamcOfBorg build vimPlugins.coc-prettier |
hmm, im just not sure about the several thousand node packages bump ... :( |
@jonringer I believe that's a side effect of node package packaging approach: https://github.com/NixOS/nixpkgs/blob/95dfbe2d63/doc/languages-frameworks/node.section.md#L37-L47 |
@jonringer, what @rvolosatovs said. I also ran this by @adisbladis in the #nixos IRC channel and they confirmed that this is normal and even desirable. I think there's a broader conversation to be had about plugin/package management policy as the "updates a million things" behavior is the standard workflow for plugins/packages in other Nixpkgs subsystems (e.g., Vim, Ruby). It's definitely surprising for people coming from outside of Nix and I'm not totally sold on it. But in the context of this particular PR, this is actually the blessed path. |
Any progress on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also change the commit history so that it's not broken if at a particular commit (you reference nodePackages.coc-prettier before it was added)
the history should reflect:
nodePackages.coc-prettier: init
vimPlugins.overrides: add coc-prettier
neovim: use TMPDIR as home during initilizaiton
@@ -2047,13 +2047,13 @@ let | |||
sha512 = "w2VwSrBoHa5BsSyH+KxEqeQBAllHhccyMFVHtGtdMpF4W7IRWfZjFiQceJPChOeTsSDVUpER2T8FA93pr0L+QA=="; | |||
}; | |||
}; | |||
"temp-0.9.0" = { | |||
"temp-0.9.1" = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change is unrelated, you added a package to the v10 list, but an entry was updated in the v12 list
@@ -16,6 +16,7 @@ | |||
, "browserify" | |||
, "castnow" | |||
, "clean-css" | |||
, "coc-prettier" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't that cause a change in pkgs/development/node-packages/node-packages-v10.nix, instead it generates a new file, which seems weird to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just as a reminder from November, I agree that this is weird, but apparently regenerating the entire node-packages-v10.nix and updating a ton of NPM packages in the process is the blessed path. I'm basing this off of 1) a conversation I had in the #nixos IRC channel and 2) the documentation.
Any update on this? |
Hey all, I got distracted from finishing this up. I'm going to incorporate @jonringer's suggestions and clean up this PR, hopefully we can get this merged soon. |
3998433
to
ffec72f
Compare
@jonringer, I've cleaned up this PR and incorporated your suggestions from November. Let me know what you think. |
Friendly ping 🙂. If we can merge this, I have a bunch of other coc extensions that I want to merge, as well. I've successfully been using this approach for ~10 extensions on my local machine via overlays. |
ffec72f
to
0a41f7a
Compare
0a41f7a
to
79ca23b
Compare
Motivation for this change
This PR adds an override for
vimPlugins.coc-prettier
as a partial fix for #64560.Depends on #72501.Previously depended on #72501, which has now been squashed into this PR.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @rvolosatovs