-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
V2 CUMULATIVE with campaign and cutoff on the 7th of july #7
base: store
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code logic is looking good to me, left some comments for coding style
export type EVENT_TYPE = "point_increase" | "user_share" | ||
export const EVENT_POINT_INCREASE: EVENT_TYPE = "point_increase" | ||
export const EVENT_USER_SHARE: EVENT_TYPE = "user_share" | ||
export type EVENT_TYPE = "point_increase" | "user_share" | "init_snapshot"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init snapshot is not used
export type EVENT_TYPE = "point_increase" | "user_share" | "init_snapshot"; | |
export type EVENT_TYPE = "point_increase" | "user_share"; |
export type EVENT_TYPE = "point_increase" | "user_share" | "init_snapshot"; | ||
export const EVENT_POINT_INCREASE: EVENT_TYPE = "point_increase"; | ||
export const EVENT_USER_SHARE: EVENT_TYPE = "user_share"; | ||
export const INIT_SNAPSHOT: EVENT_TYPE = "init_snapshot"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export const INIT_SNAPSHOT: EVENT_TYPE = "init_snapshot"; |
// removes the suffix comprised of two letters coming from POINT_SOURCE | ||
const snapshots = await ctx.store.list(AccountSnapshotLP) | ||
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | ||
return { | ||
snapshots, | ||
addresses | ||
} | ||
} | ||
export async function getAllYTSnapshots(ctx : EthContext) { | ||
// removes the suffix comprised of two letters coming from POINT_SOURCE | ||
const snapshots = await ctx.store.list(AccountSnapshotYT) | ||
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | ||
return { | ||
snapshots, | ||
addresses | ||
} | ||
} | ||
|
||
export async function getAllSYSnapshots(ctx : EthContext) { | ||
// removes the suffix comprised of two letters coming from POINT_SOURCE | ||
const snapshots = await ctx.store.list(AccountSnapshotSY) | ||
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | ||
return { | ||
snapshots, | ||
addresses | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the comment is outdated, and a suggestion we could make these three functions into a generic one with arguments for ctx and schema to make the code more concise.
export async function getAllLPSnapshots(ctx : EthContext) { | |
// removes the suffix comprised of two letters coming from POINT_SOURCE | |
const snapshots = await ctx.store.list(AccountSnapshotLP) | |
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | |
return { | |
snapshots, | |
addresses | |
} | |
} | |
export async function getAllYTSnapshots(ctx : EthContext) { | |
// removes the suffix comprised of two letters coming from POINT_SOURCE | |
const snapshots = await ctx.store.list(AccountSnapshotYT) | |
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | |
return { | |
snapshots, | |
addresses | |
} | |
} | |
export async function getAllSYSnapshots(ctx : EthContext) { | |
// removes the suffix comprised of two letters coming from POINT_SOURCE | |
const snapshots = await ctx.store.list(AccountSnapshotSY) | |
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | |
return { | |
snapshots, | |
addresses | |
} | |
} | |
export async function getAllLPSnapshots(ctx : EthContext) { | |
const snapshots = await ctx.store.list(AccountSnapshotLP) | |
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | |
return { | |
snapshots, | |
addresses | |
} | |
} | |
export async function getAllYTSnapshots(ctx : EthContext) { | |
const snapshots = await ctx.store.list(AccountSnapshotYT) | |
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | |
return { | |
snapshots, | |
addresses | |
} | |
} | |
export async function getAllSYSnapshots(ctx : EthContext) { | |
const snapshots = await ctx.store.list(AccountSnapshotSY) | |
const addresses = snapshots.map((snapshot) => snapshot.id.toString()); | |
return { | |
snapshots, | |
addresses | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion for updatePointsYT
and updatePointsYT
from the code concise pov, I think we could use one function with default timeline for both, and just pass in the SY timeline when sy is calculated
changing the comparing branch to |
No description provided.