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

Derive hash and comparison traits for RedisAlloc #386

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

iddm
Copy link
Collaborator

@iddm iddm commented Jul 25, 2024

Broadens the use of the RedisAlloc allocator by deriving more traits, so that the user can utilise the RedisAlloc in more ways.

@iddm iddm requested a review from MeirShpilraien July 25, 2024 10:13
@iddm iddm self-assigned this Jul 25, 2024
@iddm
Copy link
Collaborator Author

iddm commented Jul 25, 2024

CircleCI doesn't support macos anymore (the way we defined the job). I suggest we just merge and fix the CI later.

@@ -22,7 +22,7 @@ const REDIS_ALLOCATOR_NOT_AVAILABLE_MESSAGE: &str =
/// Defines the Redis allocator. This allocator delegates the allocation
/// and deallocation tasks to the Redis server when available, otherwise
/// it panics.
#[derive(Default, Debug, Copy, Clone)]
#[derive(Default, Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need to compare the allocator? Or hash it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't need it right here, but if the struct wants to wrap it within another struct and do some counting and simply wants to derive a trait, not having those for RedisAlloc disallows that and requires implementing it manually. This is more just for convenience.

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