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

Renamed the existing "Test" project to "Cli" and set up a new project with automated testing. #2

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

MeltyPlayer
Copy link

Hello again!

Again, assuming that this project accepts pull requests, I went ahead and added some automated code coverage. For now, these are just barebones goldens tests to make sure that audio is generated as expected.

Since there was an existing project that was already called "Test", I renamed this suffix to "Cli" instead--hopefully this is alright.

Copy link
Owner

@Monczak Monczak left a comment

Choose a reason for hiding this comment

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

I adjusted the code so it conforms to your new style guide (good thing you want everything to be consistent). Also reverted the tests to .NET 6 because the rest of the library uses that version.
Managed to fix some simple edge case bugs in my code, too :p

@Monczak
Copy link
Owner

Monczak commented Nov 19, 2023

Nice work with the automated testing thing! Coveralls is failing, though - something about an empty lcov file? I haven't used that before at all, so I'm not really sure what the proper way to work with this is. Could you look into that?

LGTM otherwise :D

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