Skip to content

Conversation

@SteampunkIslande
Copy link

That's a feature I needed, it would be nice to be able to output contaminant reads in a separate file.

This PR does exactly that, but there are a few points I'd like to discuss with the maintainers:

  • First, I made the tests pass even when mistakingly writing reads with missing line feeds. So I'd like to improve the tests to make sure I didn't break anything.
  • Second, if I understand correctly, the inverse flag of the CLI makes chopper output the 'failing' reads. Maybe we could replace this flag with dedicated file outputs.
  • And last but not least, I am a pretty bad Rust developer, so I didn't find another way of protecting concurrent writes other than that terrible Arc<Mutex<Box<dyn Write + Send>>>. There must be another simpler, more elegant technique ^^

Anyway, thanks for reading this through, have a good day :)

@wdecoster
Copy link
Owner

Hi,

Thanks for contributing! I will look at the code you wrote. Did you perhaps benchmark the time your version takes compared to the latest release?

Best,
Wouter

@SteampunkIslande
Copy link
Author

Did you perhaps benchmark the time your version takes compared to the latest release?

I'm sorry, I didn't. There might indeed be a performance cost, but overall it seems to be working just fine

@SteampunkIslande SteampunkIslande marked this pull request as draft November 21, 2024 09:27
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