neovim / neovim-ruby

Ruby support for Neovim
MIT License
340 stars 17 forks source link

vader.vim missing from release build #40

Closed jpleau closed 6 years ago

jpleau commented 7 years ago

Hi

The directory spec/integration/runtime/vader.vim/ is empty in release tarballs. It looks like it's a git submodule, but is not fully included in the release. This makes it impossible to run the tests without cloning from git!

Would it be possible to include the submodule in release tarballs?

Thanks !

alexgenco commented 7 years ago

I don't know much about releases, but I looked into it a little bit and it doesn't look like there's a way to include submodules in Github releases. I don't personally see this as a problem; if you need to run the tests you should be using a cloned repo, as the README suggests. I think the tarballs should be used for installation only.

Is there a use case here I'm missing?

jpleau commented 7 years ago

Hi.

I'll describe my use case: I am currently working on upgrading the Debian/Ubuntu package for neovim-ruby to the latest release. We have scripts that update ruby packages' source code using rubygems. (Not using git repos)

During package building (.deb), tests are ran. Now since vader.vim isn't included in the release, the tests that require it obviously fail, and package creation aborts.

I can work around it by either packaging vader.vim for Debian or disabling the tests that require it.

Hope that makes it a little bit more clear !

Thanks !

alexgenco commented 7 years ago

I just converted vader.vim from a submodule to a subtree. I'll try to release 0.5.1 tomorrow sometime, and it should include the vader.vim files.

alexgenco commented 7 years ago

I just released 0.5.1. Can you check if this works for you?

jpleau commented 7 years ago

Hi.

Thanks, that fixed the testing issue I had !

alexgenco commented 6 years ago

@jpleau I made some updates that could potentially make this an issue again. I'm using vim-themis instead of vader now, and I'm installing it using vim-flavor, instead of checking it into the repo. Can you check if things are working with this change?

jpleau commented 6 years ago

Hi, not at home today so I can't test, but I can already tell it probably will break things on my end.

Network is completely disabled on build servers, so the plugin will probably fail to be downloaded and installed.

I can simply disable tests in the packaging (not the best option, but I understand it makes things easier on your side). Maybe I can patch the rakefile at build time to only run the functionnal tests, and disable the acceptance spec.

alexgenco commented 6 years ago

Running just the functional tests seems like a good option. Do you not have any control over how it runs tests? If so, you don't have to patch the Rakefile and can just run rake spec:functional instead of rake. Themis is a significantly larger codebase than Vader (and significantly larger than neovim-ruby itself), so I'd really rather it be included lazily.

jpleau commented 6 years ago

I believe I do have control yes, not sure why I proposed to patch the Rakefile at build time instead.

I'll try to work on it this week, will report back.

Thanks for letting me know of these changes, it's appreciated !

alexgenco commented 6 years ago

Feel free to reopen if you have issues.

jpleau commented 6 years ago

Quick test: it seemed to work fine on my side. I'll wait for an official release before going further. Thanks

alexgenco commented 6 years ago

@jpleau 0.7.1 was just released, FYI

jpleau commented 6 years ago

Awesome thanks. I actually upgraded my packaging yesterday to match current git master, will update everything this week!