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

Jump to bottom

Telescope fzy native #106592

Closed
wants to merge 3 commits into from
Closed

Telescope fzy native #106592

wants to merge 3 commits into from

Conversation

onsails
Copy link
Contributor

@onsails onsails commented Dec 10, 2020 •
edited

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

@ofborg ofborg bot added the 6.topic: vim label Dec 10, 2020
@onsails
Copy link
Contributor Author

onsails commented Dec 10, 2020

Result of nixpkgs-review pr 106592 1

14 packages built:
  • spacevim
  • vimPlugins.awesome-vim-colorschemes
  • vimPlugins.coc-nvim
  • vimPlugins.defx-nvim
  • vimPlugins.nvim-highlite
  • vimPlugins.nvim-tree-lua
  • vimPlugins.nvim-treesitter
  • vimPlugins.plenary-nvim
  • vimPlugins.telescope-fzy-native-nvim
  • vimPlugins.telescope-nvim
  • vimPlugins.vim-clap
  • vimPlugins.vim-devicons
  • vimPlugins.vim-terraform
  • vimPlugins.vimtex

@onsails onsails marked this pull request as draft December 18, 2020 10:56
@onsails onsails marked this pull request as ready for review January 8, 2021 12:05
@onsails
Copy link
Contributor Author

onsails commented Jan 8, 2021

Result of nixpkgs-review pr 106592 1

38 packages built:
  • spacevim
  • vimPlugins.Tagbar (vimPlugins.tagbar)
  • vimPlugins.ale
  • vimPlugins.barbar-nvim
  • vimPlugins.brainfuck-vim
  • vimPlugins.coc-nvim
  • vimPlugins.defx-nvim
  • vimPlugins.denite (vimPlugins.denite-nvim)
  • vimPlugins.deoplete-fish
  • vimPlugins.deoplete-khard
  • vimPlugins.deoplete-nvim
  • vimPlugins.fern-vim
  • vimPlugins.fzf-vim
  • vimPlugins.julia-vim
  • vimPlugins.lh-brackets
  • vimPlugins.lh-vim-lib
  • vimPlugins.nlua-nvim
  • vimPlugins.nvim-gdb
  • vimPlugins.nvim-lspconfig
  • vimPlugins.nvim-lsputils
  • vimPlugins.nvim-scrollview
  • vimPlugins.nvim-treesitter
  • vimPlugins.nvim-treesitter-refactor
  • vimPlugins.nvim-ts-rainbow
  • vimPlugins.popfix
  • vimPlugins.snipmate (vimPlugins.vim-snipmate)
  • vimPlugins.telescope-fzy-native-nvim
  • vimPlugins.telescope-nvim
  • vimPlugins.unite (vimPlugins.unite-vim)
  • vimPlugins.vim-clap
  • vimPlugins.vim-floaterm
  • vimPlugins.vim-illuminate
  • vimPlugins.vim-lsp
  • vimPlugins.vim-snippets
  • vimPlugins.vim-visual-multi
  • vimPlugins.vim-vsnip
  • vimPlugins.vimspector
  • vimPlugins.vimtex

@ofborg ofborg bot requested a review from jorsn January 8, 2021 12:21
@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-ready-for-review/3032/434

@thiagokokada
Copy link
Contributor

Result of nixpkgs-review pr 106592 1

38 packages built:
  • spacevim
  • vimPlugins.ale
  • vimPlugins.barbar-nvim
  • vimPlugins.brainfuck-vim
  • vimPlugins.coc-nvim
  • vimPlugins.defx-nvim
  • vimPlugins.denite-nvim
  • vimPlugins.deoplete-fish
  • vimPlugins.deoplete-khard
  • vimPlugins.deoplete-nvim
  • vimPlugins.fern-vim
  • vimPlugins.fzf-vim
  • vimPlugins.julia-vim
  • vimPlugins.lh-brackets
  • vimPlugins.lh-vim-lib
  • vimPlugins.nlua-nvim
  • vimPlugins.nvim-gdb
  • vimPlugins.nvim-lspconfig
  • vimPlugins.nvim-lsputils
  • vimPlugins.nvim-scrollview
  • vimPlugins.nvim-treesitter
  • vimPlugins.nvim-treesitter-refactor
  • vimPlugins.nvim-ts-rainbow
  • vimPlugins.popfix
  • vimPlugins.tagbar
  • vimPlugins.telescope-fzy-native-nvim
  • vimPlugins.telescope-nvim
  • vimPlugins.unite-vim
  • vimPlugins.vim-clap
  • vimPlugins.vim-floaterm
  • vimPlugins.vim-illuminate
  • vimPlugins.vim-lsp
  • vimPlugins.vim-snipmate
  • vimPlugins.vim-snippets
  • vimPlugins.vim-visual-multi
  • vimPlugins.vim-vsnip
  • vimPlugins.vimspector
  • vimPlugins.vimtex

jlesquembre
jlesquembre suggested changes Jan 9, 2021
View reviewed changes
pkgs/misc/vim-plugins/overrides.nix
@@ -1,5 +1,5 @@
{ lib, stdenv
, python, cmake, meson, vim, ruby
, python, gnumake, cmake, gcc, meson, vim, ruby
Copy link
Member

Choose a reason for hiding this comment

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

Is this really necessary? gcc and make are part of the stdenv

pkgs/misc/vim-plugins/overrides.nix
${gnumake}/bin/make
'';

buildInputs = [ gcc ];
Copy link
Member

Choose a reason for hiding this comment

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

I think you can remove this line

pkgs/misc/vim-plugins/overrides.nix
telescope-fzy-native-nvim = super.telescope-fzy-native-nvim.overrideAttrs(old: {
# it already contains precompiled libraries
# rm them and build our own
preFixup = ''
Copy link
Member

Choose a reason for hiding this comment

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

make should be invoked in the the build phase, it looks a bit strange to call it in the fixup phase: https://nixos.org/manual/nixpkgs/stable/#ssec-fixup-phase

@jlesquembre
Copy link
Member

jlesquembre commented Jan 9, 2021 •
edited

I didn't see your PR and created a similar one:
#108264

Some remarks:

  • Usually is better to squash your commits when you do an update
  • In the vim-plugins/generated.nix file, if you want to add only one package, is better to add to commit only that package. That reduce the possibility of a merge conflict

@onsails
Copy link
Contributor Author

onsails commented Jan 9, 2021

I didn't see your PR and created a similar one:
#108264

Some remarks:

* Usually is better to squash your commits when you do an update

* In the vim-plugins/generated.nix file, if you want to add only one package, is better to add to commit only that package. That reduce the possibility of a merge conflict

hm, I see, thanks for your review, good to know for my future PRs!

@onsails onsails closed this Jan 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlesquembre jlesquembre jlesquembre requested changes

@thiagokokada thiagokokada thiagokokada approved these changes

@jonringer jonringer Awaiting requested review from jonringer

@jorsn jorsn Awaiting requested review from jorsn

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@onsails @nixos-discourse @thiagokokada @jlesquembre