Skip to content

Refactor Scala version check and add some tests for unsupported Scala versions #181

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

Merged
merged 1 commit into from
Dec 30, 2023

Conversation

jozic
Copy link
Collaborator

@jozic jozic commented Dec 29, 2023

No description provided.

@jozic jozic marked this pull request as ready for review December 29, 2023 05:44
@jozic
Copy link
Collaborator Author

jozic commented Dec 29, 2023

@ckipp01 if you have a couple minutes, please check it out

Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

Nice clean up!

Copy link
Member

Choose a reason for hiding this comment

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

This is all much nicer to work with. I know you have it covered with a bunch of itest, nice job adding those, but I also always find it worth the time to unit test classes like these as you often get way more coverage that way, and it's easy to miss something. But it's not a blocker.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the review!
I was on the fence myself with adding unit tests or not as there were none here , but after your comment I decided that it won't hurt to have those as well :)

@jozic jozic merged commit 051311d into master Dec 30, 2023
@jozic jozic deleted the skip-tests branch December 30, 2023 23:52
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