-
Notifications
You must be signed in to change notification settings - Fork 17
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
address failing test on windows #93
Conversation
What is this for/how does it work? Is it ready for review or should it be marked draft? |
@rbolgaryn this is still failing |
Have not been able to figure out why it is failing on Windows. I just commented out the lines that were causing the problem in the "legacy" implementation. Since it is not used other than for testing, it should be OK. This PR should be ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is just for testing and we're sure we don't need the lines that are commented out here, might as well just delete them?
remove unused code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a comment about why we aren't calculating these things here. Otherwise looks fine to me.
Fixing test for windows