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.

Sign up for GitHub

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

Jump to bottom

vimPlugins.coc-spell-checker: use a node package #189788

Merged
merged 2 commits into from Nov 17, 2022
Merged

vimPlugins.coc-spell-checker: use a node package #189788

merged 2 commits into from Nov 17, 2022

Conversation

Minion3665
Copy link
Member

@Minion3665 Minion3665 commented Sep 5, 2022 •
edited

  • coc-spell-checker needs to be from nodePackages rather than generated
    as normal vim plugins are

  • this commit adds coc-spell-checker to nodePackages and adds an
    override to the vim plugin

  • A similar pull request is vimPlugins.coc-spell-checker: Add override and node dependency #83661, although that was dropped due to conflicts && an unresponsive author

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@SuperSandro2000
Copy link
Member

LGTM, please rebase and give me a ping for a quick merge. Maybe only commit the relevant lines to reduce the amount of noise and risk not another merge conflict?

@Minion3665 Minion3665 force-pushed the coc-spell-checker branch 3 times, most recently from f339999 to 1702d51 Compare November 7, 2022 01:30
@Minion3665 Minion3665 force-pushed the coc-spell-checker branch 2 times, most recently from c12d715 to 174da23 Compare November 7, 2022 02:02
@Minion3665
Copy link
Member Author

LGTM, please rebase and give me a ping for a quick merge. Maybe only commit the relevant lines to reduce the amount of noise and risk not another merge conflict?

@SuperSandro2000 thank you so much for helping! I've rebased, regenerated all the node packages and vim plugins, and taken your advice to commit only the necessary lines.

I've run another nixpkgs-review so I believe this PR doesn't introduce any regressions and I've also tested that the coc-spell-checker plugin builds and works

@Minion3665
Copy link
Member Author

(Another rebase as node-packages.nix changes very frequently. Maybe I'll make something to just upgrade only some packages there as it seems to need it, I've also run nixpkgs-review again to make sure I'm not introducing any regressions)

@Minion3665
vimPlugins.coc-spell-checker: use a node package
54b03e5 
- coc-spell-checker needs to be from nodePackages rather than generated
  as normal vim plugins are
- this commit adds coc-spell-checker to nodePackages and adds an
  override to the vim plugin
@Minion3665
Add cspell to node-packages
5fb4db7 
- This node-packages.nix file in this commit was generated with
  ./pkgs/development/node-packages/generate.sh and then trimmed down in
  order to only commit the new package and dependencies
  Dependencies include:
  - @cspell/dict packages
  - Other cspell libraries
  - Some other JS libraries (e.g. comment-json)
- The vim/plugins/generated.nix file in this commit was generated with
  ./pkgs/applications/editors/vim/plugins/update.py and then trimmed
  down in order to only commit the removal of coc-spell-checker being
  directly in there
- This is to avoid large rebuilds (and make conflicts less likely)
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/682

@SuperSandro2000 SuperSandro2000 merged commit f635638 into NixOS:master Nov 17, 2022
@Minion3665 Minion3665 deleted the coc-spell-checker branch November 17, 2022 23:12
@figsoda figsoda mentioned this pull request Nov 18, 2022
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer Awaiting requested review from jonringer

@jagajaga jagajaga Awaiting requested review from jagajaga

@Luflosi Luflosi Awaiting requested review from Luflosi

@MarcWeber MarcWeber Awaiting requested review from MarcWeber

@midchildan midchildan Awaiting requested review from midchildan

@winterqt winterqt Awaiting requested review from winterqt

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

Assignees
No one assigned
Labels
6.topic: vim 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Minion3665 @SuperSandro2000 @nixos-discourse