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

Add luacheck and GitHub workflow #175

Open
Panquesito7 opened this issue Jan 28, 2020 · 26 comments
Open

Add luacheck and GitHub workflow #175

Panquesito7 opened this issue Jan 28, 2020 · 26 comments
Labels
good first issue The solution shouldn't be too difficult to program, and server staff will help you if need be high priority Critical issue affecting the daily functioning of the server

Comments

@Panquesito7
Copy link

See GitHub Help for more information about GitHub actions.

You need to create and configure .luacheckrc in all repositories before adding the CI, otherwise you'll get a lot of (false) errors.
If you don't know how to start, you can take inspiration from Cloud Items' .luacheckrc.

@fluxionary
Copy link
Member

What? Who's setting up CI?

@Panquesito7
Copy link
Author

You can set up CI with GitHub actions by adding a workflow.
See my first link for more information.

@fluxionary
Copy link
Member

But... CI for what? Sorry, I'm all for CI, I'm just not sure where/how it's relevant right now. This isn't a code repo.

@Panquesito7
Copy link
Author

No, I'm talking about Lua repositories.
If you need help, I can help setting it up. 🙂

I created the issue here because this is a issue tracker repository.

@fluxionary
Copy link
Member

Oh, you're just suggesting that I set up some sort of basic style checking on the lua repos and forks I manage? I suppose I'm not against playing around w/ that; I've given up on doing any real sort of continuous testing on mod repos cuz I'd need to, like, mock a full instance of the game. I also run an IDE that runs a luacheck in real time while i edit the code...

@fluxionary
Copy link
Member

I suppose I'll add this to the bls_custom repo for experimentation

@fluxionary
Copy link
Member

@Panquesito7 any comments on what I'm doing wrong here? It's kvetching about docker, and I don't know why.

https://github.com/BlockySurvival/bls_custom/actions/runs/32404553

@Panquesito7
Copy link
Author

The code has various warnings which need to be fixed.
Will check what's going on and try to fix those issues by creating a PR.

@fluxionary
Copy link
Member

Does it? The only output I see is

 Check failure on line 1 in .github

@github-actions github-actions / luacheck

.github#L1

Docker run failed with exit code 1

@Panquesito7
Copy link
Author

Panquesito7 commented Jan 29, 2020

https://github.com/BlockySurvival/bls_custom/runs/414134776
Working to fix these issues.

@Panquesito7
Copy link
Author

@Panquesito7
Copy link
Author

Now that it is working with bls_custom, I can start working with the other mods.
What do you think?

@fluxionary
Copy link
Member

I think you should go for it. I'd prefer to deal w/ the verbana mod myself, but feel free to put in PRs against the other repos we manage, and I'll set it up on the github side.

@fluxionary fluxionary added the good first issue The solution shouldn't be too difficult to program, and server staff will help you if need be label Feb 4, 2020
@Panquesito7
Copy link
Author

I've created various PRs in various repositories.
@fluxionary, you probably want to watch all BlockySurvival repositories to get all notifications.

@BuckarooBanzay
Copy link

@Panquesito7 nice work 👍

But... CI for what? Sorry, I'm all for CI, I'm just not sure where/how it's relevant right now. This isn't a code repo.

@fluxionary you can still do integration-testing for the meta-repo. We are doing that for our pandorabox-mods repository:
https://github.com/pandorabox-io/pandorabox-mods/blob/master/test.sh
https://github.com/pandorabox-io/pandorabox-mods/blob/master/.github/workflows/integration-test.yml

For every commit a minetest-instance is started that checks if the engine starts and does some basic assertions:
https://github.com/pandorabox-io/pandorabox_custom/blob/master/integration_test.lua

Background: the master branch is automatically kept in sync with the minetest-server
This helped prevent a lot of potential crashes.

The CI-Tests even run when the dependabot updates a repository and creates a PR:
pandorabox-io/pandorabox-mods#490
(this was actually @coil0's idea...)

Let me know if you need help setting that up...

@fluxionary
Copy link
Member

Thanks Buckaroo, that's really helpful. I can probably set it up myself sometime later when I get some energy.

@fluxionary fluxionary added the high priority Critical issue affecting the daily functioning of the server label May 18, 2020
@fluxionary
Copy link
Member

This is towards the top of my "high priority" list. Just need a few good days.

@BuckarooBanzay
Copy link

This is towards the top of my "high priority" list. Just need a few good days.

I tried to whip up a PR for this a few days ago but had some issues:

  • IRC-Mod needs special lua libraries
  • Mods aren't structured as worldmods folder
  • Naming issue with the REPOS folder

Those aren't blockers but the repository can't be cloned 1:1 in a worldmods folder in the current state. I guess you are working with a world.mt file that enables every mod explicitly?

@fluxionary
Copy link
Member

fluxionary commented May 18, 2020

Yes, we're using the main "mods" folder and a world.mt file to enable the mods; this allows admins to easily disable/enable mods, as well as individual submods from modpacks (and we very much do the latter). If there's something i can do to the REPOS folder & the associated symlink to make it work in a worldmods folder, w/out removing the current functionality, I'd love to hear it. Keep in mind that part of the problem is that the terumet mod's root isn't a proper mod folder itself.

@fluxionary
Copy link
Member

fluxionary commented May 18, 2020

Also, I'm fairly certain a couple other mods require special lua libraries (e.g. verbana requires a sqlite library); if we had to configure the test server to omit these handful of mods, that'd still be a huge improvement over the current situation.

@BuckarooBanzay
Copy link

Keep in mind that part of the problem is that the terumet mod's root isn't a proper mod folder itself.

We have a special case with the moretrees mod (we only symlink a few mods in the pack):
https://github.com/pandorabox-io/pandorabox-mods/tree/master/.partial_mods
All names starting with a dot are ignored by the engine.

Also, I'm fairly certain a couple other mods require special lua libraries (e.g. verbana requires a sqlite library); if we had to configure the test server to omit these handful of mods, that'd still be a huge improvement over the current situation.

You could of course just remove them at the start of the test script

@fluxionary
Copy link
Member

I'm working on refactoring things so that it'll work in either setup. Not feeling well rn though so it might be another while.

@Panquesito7
Copy link
Author

@fluxionary there are some repositories (non-meta repos) that are still missing LuaCheck.
I'll start creating more PRs when I have time.

@Panquesito7
Copy link
Author

I'm now working on to add LuaCheck for more repositories. 🙂

@fluxionary fluxionary pinned this issue Sep 2, 2020
@Panquesito7
Copy link
Author

This issue is kinda old, but I still see a few repositories with no LuaCheck.
I'll be making PRs for those. I hope that's fine. Thanks.

@fluxionary
Copy link
Member

fine by me. i've actually started using pre-commit, luacheck, and stylua in all my own projects, it certainly helps catch stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue The solution shouldn't be too difficult to program, and server staff will help you if need be high priority Critical issue affecting the daily functioning of the server
Projects
None yet
Development

No branches or pull requests

3 participants