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

Fix clippy warnings #30

Open
schoenenberg opened this issue Apr 14, 2020 · 4 comments
Open

Fix clippy warnings #30

schoenenberg opened this issue Apr 14, 2020 · 4 comments

Comments

@schoenenberg
Copy link
Contributor

Hi Pedro,

I would like to fix some clippy-warnings in the code, if this is ok for you.

Best regards,
Maximilian

@pedrocr
Copy link
Owner

pedrocr commented Apr 14, 2020

There are a few rustc warnings now that should definitely be fixed. I don't know what kinds of things clippy suggests be changed but I assume those are fine too.

One thing I haven't done yet that would be useful for those kinds of refactorings is getting an automated regression testing setup by making sure the output stays exactly the same over the test set in https://raw.pixls.us/. But feel free to suggest some changes. I'll review them manually if needed.

@schoenenberg
Copy link
Contributor Author

Regression tests would be great for a library like this. I found this library and will check it out: retest

@pedrocr
Copy link
Owner

pedrocr commented Apr 15, 2020

My plan for this was actually quite simple:

  1. Write a very simple program that takes in a file, processes it with the library and prints out all the metadata and a hash of the output data
  2. Run that program over the test set and save the output files into the repository
  3. Write a simple script that compares the output of the program with the saved files and shows any differences as test failures

Since doing the raw processing without the image pipeline is very fast, it should be a pretty fast test run that can be used on every PR/commit. The only problem is that it requires having quite a few GB of test images locally. I don't know if that means I can't do that test in travis for automation and need to set it up in my own server and integrate with GitHub.

@schoenenberg
Copy link
Contributor Author

I will definitely do this before I create a PR for the Clippy-Warnings. I'm just really busy right now. That's why it may take a while.

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

No branches or pull requests

2 participants