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

Cleanup module declaration and imports #27

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

joulei
Copy link
Collaborator

@joulei joulei commented Jan 4, 2023

What it does

Defines a uniform way to import modules, switches those messy use super::super::super::super... into always going to the root:

use crate::ecs::{
    systems::{...} 
}

Delegates the submodule declaration to each module, we used to have all modules declared in main.rs

I took a very opinionated decision here, so let me know what you think, we could declare the modules and submodules for our current codebase as follows:

Option A

     - systems // folder 
       - render_system.rs  // contains main render_system module, and sub-module declarations (`pub mod camera`)
       - physics_system.rs // same as above
       - render_sytem  // folder
           - camera.rs
        - physics_system // folder 
           - physics.rs
        ... etc.

Option B

    - system
       - render_sytem  // folder
           - mod.rs // contains main render_system module, and sub-module declarations (`pub mod camera`)
           - camera.rs
        - physics_system // folder 
           - mod.rs
           - physics.rs
        ... etc.

I went for option B, I believe having everything in the same folder makes it clearer and avoids a bunch of scrolling and clicking when navigating graphically through the codebase. It also makes module declaration feel more explicit even if the file also contains code. i.e.: you know you'll always find the mod declaration in the mod.rs file

image

I'd like to also set up a convention when organizing the imports at the top of the file, for the time being, I used:

  1. imports from our own crate
  2. imports from std
  3. imports from ggez

Unrelated, but also finally ran cargo fmt and removed /docs from the .gitignore file (so you'll see diagrams added)

Copy link

@mcass19 mcass19 left a comment

Choose a reason for hiding this comment

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

I'm from the Option A school, but always prefer this against nothing. Great work ❤️

Could be beneficial to install something like https://github.com/regexident/cargo-modules?

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