estie-inc / snowflake-connector-rs

Snowflake Connector for Rust
MIT License
20 stars 8 forks source link

Refactoring using 2018 edition style module structure #18

Closed higumachan closed 9 months ago

higumachan commented 9 months ago

Hi. I have followed the following URL to make the project structure compatible with the 2018 edition.

https://doc.rust-lang.org/edition-guide/rust-2018/path-changes.html#no-more-modrs

before

.
├── auth
│   ├── key_pair.rs
│   ├── mod.rs
│   └── test_snowflake_key.p8
├── chunk.rs
├── error.rs
├── lib.rs
├── query.rs
├── row.rs
├── session
│   └── mod.rs
└── types.rs

after

.
├── auth
│   ├── key_pair.rs
│   └── test_snowflake_key.p8
├── auth.rs
├── chunk.rs
├── error.rs
├── lib.rs
├── query.rs
├── row.rs
├── session.rs
└── types.rs

If you dare to use mod.rs, we would appreciate it if you would promptly close this PR. Thanks.


P.S. The company that manages the repository is Japanese, but is English recommended for communication on the repository?

kenkoooo commented 9 months ago

Hello, and thank you for your Pull Request. I have reservations about adopting the 2018 edition style universally, particularly when dealing with submodules within a module. While it's true that session/mod.rs can be renamed to session.rs even in such scenarios, I propose keeping it as is. The reason is that we may consider adding more modules to session in the future.

higumachan commented 9 months ago

Understood. How about the following directory structure? We can create a session directory and have a file for the currently created session, SnowflakeSession, like this:

.
├── auth
│   ├── key_pair.rs
│   └── test_snowflake_key.p8
├── auth.rs
├── chunk.rs
├── error.rs
├── lib.rs
├── query.rs
├── row.rs
├── session
│   └── snowflake_session.rs
├── session.rs
└── types.rs

If there's still a reservation about this, I think it might be better to close this PR.

kenkoooo commented 9 months ago

I believe that mod.rs remains beneficial, especially when dealing with multiple submodules within a module, as all the files of the module are located in the same directory.

higumachan commented 9 months ago

First let me thank you for your opinion. I have been pushing for edition2018 just because it is new. Thanks to you I have the opportunity to investigate what advantages edition2018 has over edition2015.

Here are the results of my investigations.

I believe that mod.rs remains beneficial, especially when dealing with multiple submodules within a module, as all the files of the module are located in the same directory.

I agree with the advantage n that its modules are encapsulated, but this is how it is written in the official documentation below.

This eliminates the special name, and if you have a bunch of files open in your editor, you can clearly see their names, instead of having a bunch of tabs named mod.rs.

In other words, edtion2015 has the disadvantage that if you have a large number of submodules, the editor tabs will be full of "mod.rs". I do remember that I had the same pain with a project of about 100 files a few time ago. This problem greatly impairs the convenience of the fuzzy finder, which is often used by modern editors.


However, we checked the following large, old projects and found that all mod.rs were left in place.

Considering the fact that it is left over in these projects, it seems unlikely that mod.rs will suddenly be disabled in edition2024, so you can compare the merits and take whichever you prefer.