Skip to content

Changed LUA module dependency#3325

Merged
ranshid merged 1 commit into
valkey-io:unstablefrom
eifrah-aws:lua-deps
Mar 9, 2026
Merged

Changed LUA module dependency#3325
ranshid merged 1 commit into
valkey-io:unstablefrom
eifrah-aws:lua-deps

Conversation

@eifrah-aws

Copy link
Copy Markdown
Contributor

Before this change, if you built valkey-server (e.g. make valkey-server) the LUA module was not built. With this change, the LUA module is now a direct a dependency of valkey-server

  • (unless BUILD_LUA=no is passed)

Added some colors to the Lua module & Lua lib Makefiles to they blend nicely in the build output.

Before this change, if you built `valkey-server` (e.g. `make valkey-server`) the LUA module
was not built. With this change, the LUA module is now a direct a dependency of `valkey-server`
- (unless `BUILD_LUA=no` is passed)

Added some colors to the Lua module & Lua lib `Makefile`s to they blend nicely
in the build output.

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
@codecov

codecov Bot commented Mar 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.92%. Comparing base (d173441) to head (31abaf2).
⚠️ Report is 27 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3325      +/-   ##
============================================
- Coverage     74.92%   74.92%   -0.01%     
============================================
  Files           129      129              
  Lines         71549    71549              
============================================
- Hits          53608    53607       -1     
- Misses        17941    17942       +1     

see 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@eifrah-aws eifrah-aws marked this pull request as ready for review March 6, 2026 15:26

@sarthakaggarwal97 sarthakaggarwal97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks!

@ranshid ranshid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM.

Not sure if this is that important to change our vended LUA Makefile (even if it yield a nicer output)

@eifrah-aws

eifrah-aws commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

@ranshid I am not sure what to do with your comment: do you want me to remove the changes from the LUA Makefiles or not? (personally, I think we should keep it, but I am getting the sense that you are against it).

@eifrah-aws eifrah-aws requested a review from ranshid March 9, 2026 08:07
@ranshid

ranshid commented Mar 9, 2026

Copy link
Copy Markdown
Member

@ranshid I am not sure what to do with your comment: do you want me to remove the changes from the LUA Makefiles or not? (personally, I think we should keep it, but I am getting the sense that you are against it).

Currently do nothing. I just wonder how much this will potentially slow us down when we will want to upgrade lua in the future. @rjd15372 WDYT?

@rjd15372

rjd15372 commented Mar 9, 2026

Copy link
Copy Markdown
Member

@ranshid I am not sure what to do with your comment: do you want me to remove the changes from the LUA Makefiles or not? (personally, I think we should keep it, but I am getting the sense that you are against it).

Currently do nothing. I just wonder how much this will potentially slow us down when we will want to upgrade lua in the future. @rjd15372 WDYT?

It should be OK. A new Lua runtime will come with its own Makefile.

@ranshid ranshid merged commit 9f01067 into valkey-io:unstable Mar 9, 2026
59 checks passed
lemire pushed a commit to lemire/valkey that referenced this pull request Mar 9, 2026
Before this change, if you built `valkey-server` (e.g. `make
valkey-server`) the LUA module was not built. With this change, the LUA
module is now a direct a dependency of `valkey-server`
- (unless `BUILD_LUA=no` is passed)

Added some colors to the Lua module & Lua lib `Makefile`s to they blend
nicely in the build output.

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
Signed-off-by: Daniel Lemire <daniel@lemire.me>
JimB123 added a commit to JimB123/valkey that referenced this pull request Mar 10, 2026
@ranshid ranshid added the release-notes This issue should get a line item in the release notes label Mar 16, 2026
JimB123 pushed a commit that referenced this pull request Mar 19, 2026
Before this change, if you built `valkey-server` (e.g. `make
valkey-server`) the LUA module was not built. With this change, the LUA
module is now a direct a dependency of `valkey-server`
- (unless `BUILD_LUA=no` is passed)

Added some colors to the Lua module & Lua lib `Makefile`s to they blend
nicely in the build output.

Signed-off-by: Eran Ifrah <eifrah@amazon.com>
@eifrah-aws eifrah-aws deleted the lua-deps branch May 11, 2026 08:26
@eifrah-aws eifrah-aws restored the lua-deps branch June 1, 2026 07:36
@eifrah-aws eifrah-aws deleted the lua-deps branch June 1, 2026 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes This issue should get a line item in the release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants