Skip to content

Conversation

@dmitrypol
Copy link
Contributor

@dmitrypol dmitrypol commented Apr 22, 2024

Scope of the changes:

  • updated example modules to reference Valkey vs Redis in variable names
  • updated tests to use valkeymodule.h
  • updated vars in tests/modules to use valkey vs redis in variable names

Summary of the testing:

  • ran make for all modules, loaded them into valkey-server and tested commands
  • ran make for test/modules
  • ran make test for the entire codebase

Dmitry Polyakovsky added 3 commits April 22, 2024 12:34
@dmitrypol dmitrypol changed the title Updated modules examples and test to use valkey Updated modules examples and tests to use valkey vs redis Apr 22, 2024
@codecov
Copy link

codecov bot commented Apr 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.34%. Comparing base (a989ee5) to head (bb1b325).

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #349      +/-   ##
============================================
- Coverage     68.37%   68.34%   -0.04%     
============================================
  Files           108      108              
  Lines         61555    61555              
============================================
- Hits          42091    42071      -20     
- Misses        19464    19484      +20     

see 13 files with indirect coverage changes

Signed-off-by: Dmitry Polyakovsky <[email protected]>
Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Great.

The exact replacements, is it just

REDISMODULE -> VALKEYMODULE
RedisModule -> ValkeyModule
redismodule -> valkeymodule

Nothing else manually edited?

Regarding running module tests, it's done with make test-modules or ./runtest-moduleapi.

@zuiderkwast zuiderkwast merged commit f0e1edc into valkey-io:unstable Apr 23, 2024
@zuiderkwast
Copy link
Contributor

I see that the CI test run the module API tests, so I think it's safe to merge. Thanks!

@dmitrypol
Copy link
Contributor Author

@zuiderkwast - correct, lots of search/replace/test carefully.

@madolson
Copy link
Member

madolson commented Apr 23, 2024

Just want to post that we no longer have a test validating that the old (RedisModule_*) modules can still be loaded. We should probably add one. @dmitrypol Do you think you can help with that?

@dmitrypol
Copy link
Contributor Author

dmitrypol commented Apr 23, 2024

@madolson - I saw this in logs when I loaded Rust module. I presume this is related? We want to support RedisModules for a while in the sprit of continuity.

50905:M 23 Apr 2024 10:19:30.434 * Legacy Redis Module .../redismodule-rs/target/debug/examples/libdmitry2.dylib found

@madolson
Copy link
Member

Yes, that is related. That is the specific log line that get's printed when a legacy Redis module is loaded.

PatrickJS pushed a commit to PatrickJS/placeholderkv that referenced this pull request Apr 24, 2024
)

Scope of the changes:
- updated example modules to reference Valkey vs Redis in variable names
- updated tests to use valkeymodule.h
- updated vars in tests/modules to use valkey vs redis in variable names

Summary of the testing:
- ran make for all modules, loaded them into valkey-server and tested
commands
- ran make for test/modules
- ran make test for the entire codebase

---------

Signed-off-by: Dmitry Polyakovsky <[email protected]>
Co-authored-by: Dmitry Polyakovsky <[email protected]>
@zuiderkwast
Copy link
Contributor

Can we have a test case that does sed s/valkey/redis/g;s/Valkey/Redis/g;s/VALKEY/REDIS/g on the modules and the test cases and then tries to compile and run them? Simple hack

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.

3 participants