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

feat: support luarocks/rocks.nvim #78

Merged
merged 1 commit into from
Jul 28, 2024

Conversation

mrcjkb
Copy link
Contributor

@mrcjkb mrcjkb commented Jul 21, 2024

Hey 👋

Summary

This PR is part of a push to get neovim plugins on luarocks.org.

See also:

With luarocks/rocks.nvim, it is the plugin authors' responsibility to declare dependencies and build instructions - not the user's.
Installing this plugin becomes as simple as :Rocks install markdown.nvim.

Things done:

  • Add a workflow that publishes tags to luarocks.org when a tag or release is pushed.

See also: this guide

Notes:

Important

with:
version: ${{ env.LUAROCKS_VERSION }}
dependencies: |
tree-sitter-markdown
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: The tree-sitter-markdown luarocks package has tree-sitter-markdown_inline as a dependency, so there's no need to specify it here.

@MeanderingProgrammer
Copy link
Owner

MeanderingProgrammer commented Jul 21, 2024

Thanks for the PR! I'm not very familiar with luarocks, but I get the general concept of moving more plugins into the repository.

I'll take a closer look at this and come back to it in a few days after I get some familiarity.

Thanks for putting this together!

Copy link
Owner

@MeanderingProgrammer MeanderingProgrammer left a comment

Choose a reason for hiding this comment

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

Thanks again for putting this together, I had a couple of clarifying questions just so I understand what's going on.

crate: tree-sitter-cli

- name: LuaRocks Upload
uses: nvim-neorocks/luarocks-tag-release@v5

Choose a reason for hiding this comment

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

Neat action :)

If we use @v7 do the labels get picked up, or is this a more general problem?

https://github.com/nvim-neorocks/luarocks-tag-release?tab=readme-ov-file#labels

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, my bad. This should be using v7.
v5 tries to run busted tests if a .busted file is present. We moved that to a separate action, because it caused issues with this one.

The labels problem is a luarocks-site bug.

Comment on lines +23 to +31
- name: Install C/C++ Compiler
uses: rlalik/setup-cpp-compiler@master
with:
compiler: clang-latest

- name: Install tree-sitter CLI
uses: baptiste0928/cargo-install@v3
with:
crate: tree-sitter-cli

Choose a reason for hiding this comment

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

I assume both of these are needed for the tree-sitter-* dependencies but I can't quite figure out why.

Like why does the environment that publishes the package need to have tree-sitter-cli available?

Do the dependencies need to be built as part of the upload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

luarocks-tag-release tries to install the package from the generated rockspec, before trying to upload (which also installs and builds dependencies).
If installing fails, it cancels the upload.

Choose a reason for hiding this comment

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

Ah, okay, that makes sense, to make sure what you upload actually works. I also now get what the Tests packaging on PR comment means. Thanks!

Copy link
Owner

@MeanderingProgrammer MeanderingProgrammer left a comment

Choose a reason for hiding this comment

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

Change looks good, I've added the api secret to the repo, should be good to go.

@MeanderingProgrammer MeanderingProgrammer merged commit 6915225 into MeanderingProgrammer:main Jul 28, 2024
@MeanderingProgrammer
Copy link
Owner

Looks like its there: https://luarocks.org/modules/MeanderingProgrammer/markdown.nvim

Added the neovim label.

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Jul 28, 2024

Thanks 🙏
Don't hesitate to ping me if you have any issues later on.

Edit:

Just tried it out with rocks.nvim:

:Rocks install markdown.nvim
:e README.md

works like a charm!

@mrcjkb mrcjkb deleted the luarocks branch July 28, 2024 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants