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

Added API to register to server events. #267

Merged
merged 25 commits into from
Mar 21, 2023
Merged

Conversation

MeirShpilraien
Copy link
Collaborator

The PR also introduce a new crate, redismodule-rs-derive, which is use to implement derive and proc macro functionality. Server events are registered using a proc_macro_attribute that add the server event handler to the relevant list server events handler list. This binding is done using: https://github.com/dtolnay/linkme

@gkorland
Copy link
Contributor

@MeirShpilraien do you need to break the crate to two crates?

@MeirShpilraien
Copy link
Collaborator Author

@gkorland Yes, derives and proc macros must be on a separate trait. This is why you have serde and serde_derive for example.

examples/server_events.rs Outdated Show resolved Hide resolved
examples/server_events.rs Outdated Show resolved Hide resolved
src/macros.rs Outdated Show resolved Hide resolved
src/context/server_events.rs Outdated Show resolved Hide resolved
src/context/server_events.rs Outdated Show resolved Hide resolved
src/context/server_events.rs Outdated Show resolved Hide resolved
src/context/server_events.rs Outdated Show resolved Hide resolved
Co-authored-by: Guy Korland <gkorland@gmail.com>
@gkorland gkorland self-requested a review January 26, 2023 14:09
@@ -27,8 +27,23 @@ jobs:
file: "Cargo.toml"
key: "package.version"
value: "${{ steps.get_version.outputs.VERSION }}"

- name: Set the version for publishing on derive crate
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably need to also edit redismodule-rs-derive?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This one is editing redismodule-rs-derive

iddm
iddm previously approved these changes Mar 13, 2023
Copy link
Collaborator

@iddm iddm left a comment

Choose a reason for hiding this comment

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

Looks good, just have a few suggestions.

src/context/server_events.rs Outdated Show resolved Hide resolved
src/context/server_events.rs Show resolved Hide resolved
src/context/server_events.rs Outdated Show resolved Hide resolved
src/context/server_events.rs Outdated Show resolved Hide resolved
tests/integration.rs Outdated Show resolved Hide resolved
MeirShpilraien and others added 2 commits March 20, 2023 12:04
Co-authored-by: Omer Shadmi <76992134+oshadmi@users.noreply.github.com>
iddm
iddm previously approved these changes Mar 21, 2023
redismodule-rs-derive/Cargo.toml Outdated Show resolved Hide resolved
redismodule-rs-derive/src/lib.rs Outdated Show resolved Hide resolved
iddm
iddm previously approved these changes Mar 21, 2023
.github/workflows/cratesio-publish.yml Show resolved Hide resolved
iddm
iddm previously approved these changes Mar 21, 2023
redismodule-rs-derive/src/lib.rs Show resolved Hide resolved
@MeirShpilraien MeirShpilraien merged commit 5e530f3 into master Mar 21, 2023
@MeirShpilraien MeirShpilraien deleted the server_events_api branch March 21, 2023 16:00
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.

5 participants