github-profile-summary-cards
github-profile-summary-cards copied to clipboard
Fixed decimal UTC time bug
fixed #90 floor down value because chart uses hours only
Thanks for your work!!
I would like to merge this PR, but maybe we can use Math.round
here?
Never mind, I am just realize that we use Math.floor
can avoid to turn 23.6 to 24, if we use Math.round
will need to write some code to deal with some special cases,.
I think we can take this PR.
I will do some more review later, to check there are no other problems.
I thought that , utc time 0 , 0.3 , 0.45 in min where 0.6 = 1 https://en.m.wikipedia.org/wiki/List_of_UTC_offsets
So for avoid dealing with 0.5 nunber and 0.3 hour / 30 min i used floor. We can add logic to round robin between hour or ceil if 0.45 . Avoided it becz it'll create un necessary complecation.
Or we can simply check value after point , 0 / 3 / 45 and update utc accordingly and use floor for default to avoid incorrect utc format
Or we can simply check value after point , 0 / 3 / 45 and update utc accordingly and use floor for default to avoid incorrect utc format
I prefer this way, but throw an exception when the UTC number is invalid. We can check if the value is invalid and throw an exception here to alert the caller the UTC format was incorrect.
And like you said, default to using floor.
When i looked chart i realise we need to use number.toFixed(2) // '5.30'
this or treat utc as string bcz in chart time should display as e.g. +5.30 or -5.30
But now its just shows 5.3 .
Both are same but first glance it look like something is wrong with utc time .
You are right, would you like to fix it in this PR?
Or we can create another issue for this if that need to modify many codes.
Seting up utc time on chart as well handling offset simple task , but I'm not sure how will you display warning or handle the wrong utc and dont want to lean whole code. I'll handle chart display and utc offset u do something about warning .
Sounds good to me, thanks!
By the way, about the warning, I will just throw an exception like this.
if(!IsValidUTCTime(utcTime){
throw Exception ("The UTC format is invalid, it should be ......")
}
const adjustOffset = function (offset: number, RoundRobin: {offset: number}): number {
if (offset % 1 == 0) {
return offset;
// offset % 1 should be 0.3 or 0.7 but its js and it gives 0.29999 or -0.299999 thats why this frankenstein
} else if ((offset % 1 > 0.29 && offset % 1 < 0.31) || (offset % 1 < -0.29 && offset % 1 > -0.31)) {
// toggle up and down between hour
RoundRobin.offset = (RoundRobin.offset + 1) % 2;
return RoundRobin.offset === 0 ? Math.floor(offset) : Math.ceil(offset);
} else if ((offset % 1 > 0.44 && offset % 1 < 0.46) || (offset % 1 < -0.44 && offset % 1 > -0.45)) {
// distrubute 1 : 3 ratio for 0.45 utc time
RoundRobin.offset = (RoundRobin.offset + 1) % 4;
return RoundRobin.offset % 4 == 0 ? Math.floor(offset) : Math.ceil(offset);
} else {
// flood down , if utc is given right it will never be executed
return Math.floor(offset);
}
};
.
.
let RoundRobin = {
offset: 0
};
.
.
chartData[24 + adjustOffset(afterOffset, RoundRobin)] += 1;
or use simply this for 0.3 and 0.45
Math.ceil(afterOffset);
By the way, about the warning, I will just throw an exception like this.
if(!IsValidUTCTime(utcTime){ throw Exception ("The UTC format is invalid, it should be ......") }
in api IDK how it will display
const adjustOffset = function (offset: number, RoundRobin: {offset: number}): number { if (offset % 1 == 0) { return offset; // offset % 1 should be 0.3 or 0.7 but its js and it gives 0.29999 or -0.299999 thats why this frankenstein } else if ((offset % 1 > 0.29 && offset % 1 < 0.31) || (offset % 1 < -0.29 && offset % 1 > -0.31)) { // toggle up and down between hour RoundRobin.offset = (RoundRobin.offset + 1) % 2; return RoundRobin.offset === 0 ? Math.floor(offset) : Math.ceil(offset); } else if ((offset % 1 > 0.44 && offset % 1 < 0.46) || (offset % 1 < -0.44 && offset % 1 > -0.45)) { // distrubute 1 : 3 ratio for 0.45 utc time RoundRobin.offset = (RoundRobin.offset + 1) % 4; return RoundRobin.offset % 4 == 0 ? Math.floor(offset) : Math.ceil(offset); } else { // flood down , if utc is given right it will never be executed return Math.floor(offset); } }; . . let RoundRobin = { offset: 0 }; . . chartData[24 + adjustOffset(afterOffset, RoundRobin)] += 1;
I think we can take this code, and after that, we don't need to throw exceptions, tho.
By the way, about the warning, I will just throw an exception like this.
if(!IsValidUTCTime(utcTime){ throw Exception ("The UTC format is invalid, it should be ......") }
in api IDK how it will display
We don't care about the error display here, just throw an exception.
And Math.ceil(afterOffset);
is accepted too, if we use this way, I will take the UTC validation part.
@SauravDharwadkar Thanks for your hard work!
Are you still working on this? Please let me know if there is any problem.
was busy . i didn't test negative value bcz process.arg not work with negative value
Hi, @SauravDharwadkar. Are you still working on this? Since this PR is almost done, if you don't mind I can take over the rest part and keep your commit in the commit history.
You can take over , sorry for late reply busy last couple of months
Solved in #123