Bug 250802 – Remove duplicate isLeapYear

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Ahmad Saleem]

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!

[Brent Fulgham]

Yes! Use the one in WTF/DateMath.h and delete the other one.

[Alexey Proskuryakov]

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)

[Ahmad Saleem]

(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?

[Ahmad Saleem]

(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. :-)

[Alexey Proskuryakov]

> 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.

[Ahmad Saleem]

(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. :-)

[Ahmad Saleem]

PR - https://github.com/WebKit/WebKit/pull/8806

[EWS]

Committed 259080@main (0b9c23b10f38): <https://commits.webkit.org/259080@main>

Reviewed commits have been landed. Closing PR #8806 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/104425042>