Fix export and version comparison #43
Conversation
|
|
||
| function compareVersion(version1, version2) { | ||
| var result=false; | ||
| for(var i=0;i<(Math.max(version1.length, version2.length));i++) { |
There was a problem hiding this comment.
shouldn't it be Math.min? You don't want to access an index greater than the string length, do you? :-)
There was a problem hiding this comment.
Yes we do.
If we're comparing 9.0.0 and 9.0.0.1 we still need to check the last value. But I need to add a safety conversion if NaN or undefined.
| var result=false; | ||
| for(var i=0;i<(Math.max(version1.length, version2.length));i++) { | ||
| if(Number(version1[i])<Number(version2[i])) { | ||
| result=true; |
| result=true; | ||
| break; | ||
| } | ||
| if(version1[i]!==version2[i]) { |
There was a problem hiding this comment.
if(version1[i] !== version2[i]) {
return false; // version 2 is higher
}Otherwise: do nothing and continue with the next digit.
There was a problem hiding this comment.
Yeah, I snapped this code from somewhere and didn't even realised how poorly it was written...
Please forgive me! 😢
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
skjnldsv
force-pushed
the
fix-version-compare
branch
from
f67ff10 to
4b05574
Compare
|
@ChristophWurst Fixed and rewritten 😊 |
|
nice! code looks good now |
|
you could add a unit test for this function 😉 |
|
I could I could... 😂 |
|
merge? 🙈 🙊 🙉 |
|
Since it's a function inside a controller, I don't think I can do tests directly :) |
You could export but prefix it with an underscore ;-) |
|
But is it really necessary? :D |
|
Thanks for fixing this. I'm still trying to figure out how this version comparison will solve the export of single address groups. But I am eager to give it a try. Will this be released alongside NC 11? Or how can I try this best? |
skjnldsv
added
4. to release
We can have the export back! 🎉
fix #39
@irgendwie @brjhaverkamp