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

[Feature]Grouping modules #304

Open
ArtemIsmagilov opened this issue Oct 21, 2024 · 1 comment
Open

[Feature]Grouping modules #304

ArtemIsmagilov opened this issue Oct 21, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@ArtemIsmagilov
Copy link
Contributor

ArtemIsmagilov commented Oct 21, 2024

Describe the solution you'd like
I would like to suggest regrouping modules, tests and other components into groups.

I think it will be easier to navigate the commands and write tests.
The documentation already indicates a clear separation.

What is my problem? For example, I want to add the ECHO command and ACL DRYRUN. Looking at the documentation, I see that they belong to the Connection group. Great, tests and functionality can be written side by side and know that they correspond. But in this library, ACL commands belong to the acl module and the ECHO command to connection. I will say right away that the existing version is elegant enough to be divided into entities. However, I suggest reworking it in the following way.

Rename the modules to src/commands/impls/connection_group.rs and src/commands/interfaces/connection_group.rs with grouping other commands acl, client...
By analogy, also tests and Cargo configuration.

It may seem like a lot of work and the result is small. I think that the corresponding division according to official canons allowed not to confuse also in traits, and in teams, and in tests. Plus to this we know for sure that no team exists outside the group/category for both redis and valkey

It would be great to group traits like this. For example, you have separate traits Client, ACL, AUTH. I suggest combining them into one trait called ConnectionCategory/ConnectionInterface in accordance with the documentation.

It would be cool to know your opinion on this proposal, with respect, Artem)

Additional context

Command Categories
auth trait
client trait
acl trait

@ArtemIsmagilov ArtemIsmagilov added the enhancement New feature or request label Oct 21, 2024
@ArtemIsmagilov
Copy link
Contributor Author

ArtemIsmagilov commented Oct 21, 2024

I spent a lot of time to figure out where it is better to add what. This adjustment is absolutely necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants