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

Proposal: Refactor source file structure #977

Closed
2 tasks done
PragmaTwice opened this issue Oct 12, 2022 · 12 comments · Fixed by #989
Closed
2 tasks done

Proposal: Refactor source file structure #977

PragmaTwice opened this issue Oct 12, 2022 · 12 comments · Fixed by #989
Labels
enhancement type enhancement

Comments

@PragmaTwice
Copy link
Member

PragmaTwice commented Oct 12, 2022

Search before asking

  • I had searched in the issues and found no similar issues.

Motivation

As mentioned in #973 (review), we can categorize source files in to different folders to modularize the source structure.

Solution

The preliminary plan is as follows:

common/
  event_util fd_util parse_util cron encoding db_util event_util log_collector
  parse_util rw_lock status tls_util util

database/
  batch_extractor compact_filter compaction_checker event_listener geohash
  lock_manager redis_db redis_disk redis_metadata redis_pubsub
  scripting stats storage table_properties_collector

service/
  server worker redis_reply redis_connection redis_request

distributed/
  cluster redis_slot replication slot_import slot_migrate 

data_types/
  redis_bitmap redis_bitmap_string redis_geo redis_list redis_hash redis_set
  redis_sortedint redis_stream_base redis_stream redis_string redis_zset

commands/
  redis_cmd (need to be divided)

config/
  config config_type config_util
  
crypto/
  rand sha1 rocksdb_crc32c

Since I am not very familiar with certain parts of the code, I welcome everyone's thoughts, comments and suggestions!

cc @git-hulk @ShooterIT @caipengbo @tisonkun @torwig

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@PragmaTwice PragmaTwice added the enhancement type enhancement label Oct 12, 2022
@tisonkun
Copy link
Member

Will version.h.in leave as is? I'm thinking of moving VERSION file in the src folder so that we have a shorter top-level file list.

@PragmaTwice
Copy link
Member Author

PragmaTwice commented Oct 12, 2022

In my thought, these source folders are all in the src directory.

I am not sure where to put version.h.in now, maybe just in src/.

@tanruixiang
Copy link
Member

I think the separation of redis_cmd can be done by referring to the official command classification of redis.
https://redis.io/commands/

@PragmaTwice
Copy link
Member Author

PragmaTwice commented Oct 13, 2022

Hi everyone, how do you like this proposal?

I think the current source files are cluttered in one directory, and as the source files become more and more numerous, it becomes more and more inconvenient to manage & locate them in one directory.

If you like this, you can give your thoughts and opinions on the current file classification to avoid me misclassifying some source files.

@git-hulk
Copy link
Member

git-hulk commented Oct 13, 2022

@PragmaTwice I have some proposals about naming:

  1. distributed vs cluster - distributed looks too general and those files are cluster related
  2. data_types vs types - types is intuitive enough?
  3. database vs storage - I think storage is more precise than database
  4. service vs network

and seems a bit confused that putting crc32 and rand in crypto.

@git-hulk
Copy link
Member

see whether folks have other suggestions.

@PragmaTwice
Copy link
Member Author

@PragmaTwice I have some proposals about naming:

  1. distributed vs cluster - distributed looks too general and those files are cluster related
  2. data_types vs types - types is intuitive enough?
  3. database vs storage - I think storage is more precious than database
  4. service vs network

and seems a bit confuse that puts crc32 and rand in crypto.

Greate advices, I will follow them later.

@caipengbo
Copy link
Contributor

@PragmaTwice I have a few ideas of my own:

  • crypto seem to fit under common(or util)?
  • commands may be split up and placed under each data type?

@PragmaTwice
Copy link
Member Author

PragmaTwice commented Oct 14, 2022

Thanks. Here is the second edition: cc @git-hulk @caipengbo

common/
  event_util fd_util parse_util cron encoding db_util event_util log_collector
  parse_util rw_lock status tls_util util rand sha1 rocksdb_crc32c

storage/
  batch_extractor compact_filter compaction_checker event_listener
  lock_manager redis_db redis_disk redis_metadata redis_pubsub
  scripting stats storage table_properties_collector geohash

network/
  server worker redis_reply redis_connection redis_request

cluster/
  cluster redis_slot replication slot_import slot_migrate 

types/
  redis_bitmap redis_bitmap_string redis_geo redis_list redis_hash redis_set
  redis_sortedint redis_stream_base redis_stream redis_string redis_zset

commands/
  redis_cmd (need to be divided)

config/
  config config_type config_util

./
  version main

commands may be split up and placed under each data type?

Great idea, I think we can discuss this after this classification.

@git-hulk
Copy link
Member

Looks good to me

@caipengbo
Copy link
Contributor

LGTM

@PragmaTwice
Copy link
Member Author

Thanks all. I will prepare a PR soon.

@PragmaTwice PragmaTwice linked a pull request Oct 14, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement type enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants