Bug 250802

Summary: Remove duplicate isLeapYear
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: Web Template FrameworkAssignee: Ahmad Saleem <ahmad.saleem792>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bfulgham, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   

Description Ahmad Saleem 2023-01-18 15:54:54 PST 
Hi Team,

I came across another refactoring patch to remove duplicate function:

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/ecbe8115cc1f014eb61dff29b96e405a0ab3ef51

WebKit Source - https://searchfox.org/wubkat/source/Source/WTF/wtf/DateMath.h#188 & https://searchfox.org/wubkat/source/Source/WebCore/platform/DateComponents.cpp#56

We have two functions exactly same, one is inline while another is static.

Can we get rid of one?

Thanks!
Comment 1 Brent Fulgham 2023-01-18 16:23:47 PST 
Yes! Use the one in WTF/DateMath.h and delete the other one.
Comment 2 Alexey Proskuryakov 2023-01-18 16:35:47 PST 
It would be nice to move other generic functions as well, and unify duplicates (like daysInMonths in ISO8601.cpp). But that would be a much larger effort.

static int maxDayOfMonth(int year, int month)
static int dayOfWeek(int year, int month, int day)
Comment 3 Ahmad Saleem 2023-01-18 16:36:59 PST 
(In reply to Alexey Proskuryakov from comment #2)
> It would be nice to move other generic functions as well, and unify
> duplicates (like daysInMonths in ISO8601.cpp). But that would be a much
> larger effort.
> 
> static int maxDayOfMonth(int year, int month)
> static int dayOfWeek(int year, int month, int day)

Should I create a bug for future?
Comment 4 Ahmad Saleem 2023-01-18 16:37:12 PST 
(In reply to Brent Fulgham from comment #1)
> Yes! Use the one in WTF/DateMath.h and delete the other one.

Happy to do PR. :-)
Comment 5 Alexey Proskuryakov 2023-01-18 16:47:27 PST 
> Should I create a bug for future?

I personally wouldn't bother (refactoring ideas don't usually get acted on), but it wouldn't be unreasonable to do if you feel like it.
Comment 6 Ahmad Saleem 2023-01-18 16:54:12 PST 
(In reply to Alexey Proskuryakov from comment #5)
> > Should I create a bug for future?
> 
> I personally wouldn't bother (refactoring ideas don't usually get acted on),
> but it wouldn't be unreasonable to do if you feel like it.

Not in near future (few weeks), still learning few bits around WebKit but in few months, I can explore it as a challenge. I will add it in my To-do Notes. :-)
Comment 7 Ahmad Saleem 2023-01-18 17:07:57 PST 
PR - https://github.com/WebKit/WebKit/pull/8806
Comment 8 EWS 2023-01-19 05:36:50 PST 
Committed 259080@main (0b9c23b10f38): <https://commits.webkit.org/259080@main>

Reviewed commits have been landed. Closing PR #8806 and removing active labels.
Comment 9 Radar WebKit Bug Importer 2023-01-19 05:37:16 PST 
<rdar://problem/104425042>