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

ycmd: 2019-09-19 -> 2020-02-22 #81034

Merged
merged 2 commits into from Feb 27, 2020
Merged

ycmd: 2019-09-19 -> 2020-02-22 #81034

merged 2 commits into from Feb 27, 2020

Conversation

evanjs
Copy link
Member

@evanjs evanjs commented Feb 25, 2020

Motivation for this change

It seems YCMD was left behind (from YCM) with #80218.
This normally might not make a difference, but this time, they came out of sync with commits that dropped python2 support.

The commits mentioned are ycm-core/YouCompleteMe#3566 and ycm-core/ycmd#1382, both of which were merged within the last month.

This broke YouCompleteMe, throwing the message:
YouCompleteMe unavailable: No module named 'future'

Things done

This PR updates YCMD and disables python2 support for both YCM and YCMD.

  • 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.

@rasendubi
Copy link
Member

The diff looks good, but does last commit need to go with ycmd update? It looks like it touches quite some things beside ycmd

@evanjs
Copy link
Member Author

evanjs commented Feb 27, 2020

The diff looks good, but does last commit need to go with ycmd update? It looks like it touches quite some things beside ycmd

Assuming that vimPlugins is to be updated with PRs pertaining to any packages in the set, then yes.

I know we need to update the set for plugin additions, though I’m not sure about plugin updates.
Vim Section Docs

This is assuming that the change for dropping python2 support for youcompleteme should be done in this PR, anyway.
As YCMD is a dependency of YCM, I felt this was acceptable.

@evanjs evanjs force-pushed the ycmd-2020-02-22 branch 2 times, most recently from 460dd60 to cfdcd0a Compare February 27, 2020 22:27
@evanjs
ycmd: 2019-09-19 -> 2020-02-22
cca60b0 
drop support for python2

ycmd dropped support for python2 with ycm-core/ycmd#1382

youcompleteme dropped support for python2 with ycm-core/YouCompleteMe#3566
@evanjs
vimPlugins: update
9ec772d
jonringer
jonringer approved these changes Feb 27, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM
commit LGTM

[41 built, 25 copied (193.7 MiB), 44.9 MiB DL]
https://github.com/NixOS/nixpkgs/pull/81034
36 package built:
vimPlugins.SpaceCamp vimPlugins.ale vimPlugins.awesome-vim-colorschemes vimPlugins.coc-git vimPlugins.coc-java vimPlugins.coc-metals vimPlugins.defx-nvim vimPlugins.denite-git vimPlugins.deoplete-fish vimPlugins.deoplete-khard vimPlugins.deoplete-nvim vimPlugins.echodoc-vim vimPlugins.falcon vimPlugins.jedi-vim vimPlugins.ncm2-ultisnips vimPlugins.neosnippet-snippets vimPlugins.nvim-lsp vimPlugins.seoul256-vim vimPlugins.ultisnips vimPlugins.unicode-vim vimPlugins.vim-airline vimPlugins.vim-codefmt vimPlugins.vim-dirvish vimPlugins.vim-fugitive vimPlugins.vim-gitgutter vimPlugins.vim-go vimPlugins.vim-localvimrc vimPlugins.vim-slime vimPlugins.vim-snippets vimPlugins.vim-terraform vimPlugins.vim-test vimPlugins.vim-visual-multi vimPlugins.vimtex vimPlugins.vista-vim vimPlugins.youcompleteme ycmd

@jonringer jonringer merged commit 6bb6882 into NixOS:master Feb 27, 2020
@jonringer
Copy link
Contributor

@rasendubi, @evanjs is right, it's common practice to update all vimplugins before adding a new plugin

@evanjs
Copy link
Member Author

evanjs commented Feb 28, 2020

@rasendubi, @evanjs is right, it's common practice to update all vimplugins before adding a new plugin

Thanks for clarifying! I wasn’t certain if this also applies to updates!

@evanjs evanjs deleted the ycmd-2020-02-22 branch February 28, 2020 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer approved these changes

@LnL7 LnL7 Awaiting requested review from LnL7

@jagajaga jagajaga Awaiting requested review from jagajaga

@cstrahan cstrahan Awaiting requested review from cstrahan

@rasendubi rasendubi Awaiting requested review from rasendubi

@jorsn jorsn Awaiting requested review from jorsn

Assignees
No one assigned
Labels
6.topic: vim 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
3 participants
@evanjs @rasendubi @jonringer