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

Add Input Type Autodetection and FFT CLI Argument #46

Merged
merged 6 commits into from
Oct 16, 2023

Conversation

Daniil-Golikov
Copy link
Contributor

@Daniil-Golikov Daniil-Golikov commented Oct 13, 2023

Summary:
This pull request introduces input type autodetection and adds the FFT as a CLI argument. It improves the error handling mechanism by propagating errors instead of using .expect(), and ensures the right number of arguments is provided by the user.

Changes:

  • Input Type Autodetection:

Implemented a mechanism to autodetect the input type. The program now automatically identifies the type of the input, reducing the need for manual selection.

  • FFT CLI Argument:

Added FFT as a CLI argument.

  • Arguments Count Validation:

Integrated a check for the number of provided arguments. This addition ensures users provide the correct number of arguments.

  • Error Propagation:

Transitioned from using .expect() to an error propagation.

Any feedback is welcome!

- Implement input type autodetection
- Add FFT as a CLI argument
- Check the number of provided arguments
- Implement input type autodetection
- Add FFT as a CLI argument
- Check the number of provided arguments
- Better error handling
…file-directory-autodetect

# Conflicts:
#	brro-compressor/src/main.rs
@rukai
Copy link
Contributor

rukai commented Oct 16, 2023

Heads up that you have a merge conflict here that needs to be resolved.

@Daniil-Golikov Daniil-Golikov requested a review from rukai October 16, 2023 22:34
@rukai
Copy link
Contributor

rukai commented Oct 16, 2023

mind resolving the new conflict? 😄
I figured it would be better to give you the conflict since carlos is asleep.

@Daniil-Golikov
Copy link
Contributor Author

mind resolving the new conflict? 😄 I figured it would be better to give you the conflict since carlos is asleep.

Absolutely, no problem! I've gone ahead and resolved the conflicts

Copy link
Contributor

@rukai rukai left a comment

Choose a reason for hiding this comment

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

Removing the need for the user to redundantly specify file or directory is a really nice improvement to usability!

Returning a useful error when the user gives invalid compressor type inputs is also nice.
In a follow up PR lets take this one step further, we can provide an interface to the user such that its impossible to provide multiple compression types by making a force_compression: Compressor field on Args.
You can refer to this as an example: https://github.com/shotover/shotover-proxy/blob/02c8b120d661f0e007182d5afb9c96796b658044/shotover/src/runner.rs#L47-L55

@rukai rukai merged commit 75d2ddd into main Oct 16, 2023
2 checks passed
@Daniil-Golikov Daniil-Golikov deleted the file-directory-autodetect branch October 18, 2023 10:37
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