-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
WIP Rewrite selection code #870
base: master
Are you sure you want to change the base?
Conversation
See #859 that found the issue. Tests are currently hard-code to LF so they fail on windows. I presume will work here on *nix but haven't confirmed at time of writing. I used the build by skipping tests on mybatis which was taking minutes to process, now seems faster but need to do more testing to confirm. @mathieucarbou Can you look this over to see if I'm on the right path here? |
ok ignore my mybatis test results. Variance is a windows thing so it doesn't actually improve with this for that project. Seems rather something on my machine changed so I was getting better results but retesting its master I get seconds of same result so this has no improvement there. Will it improve projects with node_modules, probably but its a read so I doubt it changes much. I still think original logic is indeed incorrect but not sure what was in mind back in 2020 and regardless issue I thought this solved I think is longer standing than 2020. |
ok GHA confirmed. Tests need simply updated to work cross platform. Provided this is on right track, will address the tests afterwards. *nix worked fine per GHA. |
Code is invalid, confirmed on mybatis project that licensing incredibly speeds up by fixing this and still does what it should. Tests currently fail as they are hard-coded to LF not OS dependent. Need to fix that but pushing up to get a review.
from using profiler with maven.
Code is invalid, confirmed on mybatis project that licensing incredibly speeds up by fixing this and still does what it should. Tests currently fail as they are hard-coded to LF not OS dependent. Need to fix that but pushing up to get a review.