steinwurf / cpuid

C++ library for detecting CPU capabilities
https://cpuid.steinwurf.com
Other
105 stars 21 forks source link

Impl split #6

Closed mortenvp closed 10 years ago

mortenvp commented 10 years ago

@nestorjhernandezm I tried an alternative way of splitting the implementation.. I think it works pretty well.

nestorjhernandezm commented 10 years ago

@mortenvp

Great! I have been taking a look and it seems nice. Actually this is what I was thinking about to do at the very end to clean up the code, but after the "all green" builds which is what I'm about to finish. It is better because it helps to keep modularity and readability as we have discussed.

About the failed tests, I'll post an answer on the mailing list with details, but I have to go to the university first. I think the better here is just to set pre-defined false values as test parameters in each CPU when you don't have a capability. Jeppe gave me a hand here, so it's making some changes in the buildbot.

mortenvp commented 10 years ago

ok, so I will merge this one

mortenvp commented 10 years ago

@nestorjhernandezm just did some final changes - review if you like then merge. The buildbot builds the project on all platforms only the tests fail - because of wrong or missing arguments.

nestorjhernandezm commented 10 years ago

@mortenvp

Its great, definitively the way it should be. If just in the other approach there would had been few differences between each implementation then that approach would have been better. At the very end, since there were several variants, a single source file was hard to read. Good, let's merge this one. I like it :+1: