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

refactor: Split command/transcode.rs into discrete unit testable functions #27

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

RogueOneEcho
Copy link

@RogueOneEcho RogueOneEcho commented May 19, 2024

Notes

This PR follows on from #26

I'm new to rust and figured refactoring an existing project would be a useful exercise to learn the language - as such I'd recommend a thorough review as there may obvious things I've missed or am not aware of.

As it's a significant revision I'd recommend reviewing it commit-by-commit as each is a discrete refactoring

Notable changes

Mostly the changes are just extracting logic but these are worth noting in particular:

  • bc9083e - changes the logic for how "has no FLAC base to transcode from... skipping" is determined.

Previous behaviour skipped if there was no existing FLAC release, but this change ensure the specified torrent is FLAC which seemed more logical.

  • 8b0aec4 - keeps TranscodeCommand as immutable and passes it by reference

This avoids partial moves without resorting to cloning and seems to be the more rust-like way of doing things

Testing

For the tests to work you will need to add some .flac files to samples/flacs/[TITLE]/[file_name].flac

To get tests running in CI some Open Source / Creative Commons could be downloaded and a mock api response created for them.

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.

None yet

1 participant