Paul/paginate caching #184
Conversation
paullinator
force-pushed
the
paul/paginateCaching
branch
from
7418602 to
9e03bd8
Compare
paullinator
force-pushed
the
paul/paginateCaching
branch
2 times, most recently
from
94a452a to
9e03bd8
Compare
src/cacheEngine.ts
Outdated
| app.appId, | ||
| appAndPartnerId, | ||
| timePeriod | ||
| 'hour,day,month' |
There was a problem hiding this comment.
What if TIME_PERIODS.join(',')?
src/util.ts
Outdated
| export const getStartOfMonthsAgo = ( | ||
| dateString: string, | ||
| months: number | ||
| ): Date => { | ||
| const date = new Date(dateString) | ||
| const result: Date = new Date( | ||
| Date.UTC(date.getUTCFullYear(), date.getUTCMonth() - months, 1) | ||
| ) | ||
| return result | ||
| } | ||
|
|
||
| export const getStartOfMonthsFromNow = ( | ||
| dateString: string, | ||
| months: number | ||
| ): Date => { | ||
| const date = new Date(dateString) | ||
| const year = date.getUTCFullYear() | ||
| const month = date.getUTCMonth() | ||
| return new Date(Date.UTC(year, month + months, 1, 0, 0, 0, 0)) | ||
| } |
There was a problem hiding this comment.
Why in one function do you have more args to Date.UTC and why not keep consistent with the pattern of inlining getUTCFullYear/getUTCMonth calls?
There was a problem hiding this comment.
I think because I created getStartOfMonthsAgo as a copy from elsewhere in the code, and when I created getStartOfMonthsFromNow, I didn't like the difficulty reading the args inline so I broke them out. Just never changed getStartOfMonthsAgo
src/cacheEngine.ts
Outdated
| const birthdayStart = getStartOfMonthsAgo( | ||
| new Date('2017-01Z').toISOString(), | ||
| 0 | ||
| ).toISOString() |
There was a problem hiding this comment.
I think we're using this 2017 date commonly as a epoch birthday of some kind that we start querying from or back to... Maybe it would make sense to move these constants to a file of all the various types of these dates (string, ms timestamp, second timestamps, date object, etc), and then we can comment on the reasoning behind this 2017 date. That'd be helpful for documentation purpose and re-use.
There was a problem hiding this comment.
Sort of, but I do change the date for different plugins since I don't got as far back as 2017 if I for sure know that we added the provider in 2022.
src/cacheEngine.ts
Outdated
There was a problem hiding this comment.
Why do we do this? Add reasoning to the commit message.
paullinator
force-pushed
the
paul/paginateCaching
branch
3 times, most recently
from
6f92b13 to
8f3d3b4
Compare
|
/rebase |
github-actions
bot
force-pushed
the
paul/paginateCaching
branch
from
8f3d3b4 to
5f7f2a3
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
none