Skip to content

Small cleanup after #74 - #75

Merged
gabotechs merged 4 commits into
mainfrom
gabrielmusat/port_and_refactor_execution_code_suggestions
Aug 6, 2025
Merged

Small cleanup after #74#75
gabotechs merged 4 commits into
mainfrom
gabrielmusat/port_and_refactor_execution_code_suggestions

Conversation

@gabotechs

@gabotechs gabotechs commented Aug 5, 2025

Copy link
Copy Markdown
Collaborator

Some small cleanup after #74:

  • removes some unused imports
  • removes an empty resut.rs file
  • uncomments the tests in favor of ignoring them
  • moves tcph/mod.rs -> to common/tpch.rs following the Rust book example for integration tests

Comment thread src/common/mod.rs

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This file seems to be empty, I just deleted it

Comment thread tests/common/plan.rs

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

As instead of commenting the tests I have #[ignore]-ed them, this file was still needed, otherwise a compilation error will happen.

Comment thread tests/common/tpch.rs

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Following the Rust book recommendation https://doc.rust-lang.org/rust-by-example/testing/integration_testing.html, I moved all the common modules that are meant to be used by the integration tests to common/

#[allow(dead_code)]
mod common;
/*

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Instead of having big chunks of commented code, a more Rusty way of handling this is to just #[ignore] the tests

Base automatically changed from robtandy/port_and_refactor_execution_code to main August 5, 2025 10:58
@gabotechs
gabotechs force-pushed the gabrielmusat/port_and_refactor_execution_code_suggestions branch from eef7375 to e4e2dbb Compare August 5, 2025 11:05

@NGA-TRAN NGA-TRAN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@gabotechs
gabotechs merged commit c32a86f into main Aug 6, 2025
3 checks passed
@gabotechs
gabotechs deleted the gabrielmusat/port_and_refactor_execution_code_suggestions branch August 6, 2025 16:38
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.

3 participants