Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Airdrop Claims Migration - #1 - Arbitrum - Ready for review #4210

Merged
merged 4 commits into from
Sep 6, 2023
Merged

Airdrop Claims Migration - #1 - Arbitrum - Ready for review #4210

merged 4 commits into from
Sep 6, 2023

Conversation

henrystats
Copy link
Contributor

Thank you for contributing to Spellbook!

Please refer to the top of the readme in the root of Spellbook to learn how to contribute to Spellbook on DuneSQL.

@dune-eng
Copy link

Workflow run id 6030364112 approved.

@dune-eng
Copy link

Workflow run id 6030364070 approved.

@dune-eng
Copy link

dune-eng commented Sep 2, 2023

Workflow run id 6058176010 approved.

@dune-eng
Copy link

dune-eng commented Sep 2, 2023

Workflow run id 6058176259 approved.

@dune-eng
Copy link

dune-eng commented Sep 3, 2023

Workflow run id 6061532003 approved.

@dune-eng
Copy link

dune-eng commented Sep 3, 2023

Workflow run id 6061531998 approved.

@henrystats henrystats changed the title Airdrop Claims Migration - #1 - Arbitrum Airdrop Claims Migration - #1 - Arbitrum - Ready for review Sep 3, 2023
@antonio-mendes
Copy link
Contributor

antonio-mendes commented Sep 6, 2023 •
edited

Starting to go through these @jeff-dude is there any particular reason why these models were separated between the blockchain folder and the airdrop folder rather than having everything inside airdrop with subfolders per chain? Seems to me like it'd be more organised that way.

If you agree I'll do the change for all these PRs before we merge! But it's no big deal or blocker

EDIT: Ignore the above! I see now it's because of the ARB airdrop specifically

@antonio-mendes
Copy link
Contributor

Thanks again for the work on these @henrystats , will try to get them merged ASAP

antonio-mendes
antonio-mendes reviewed Sep 6, 2023
View reviewed changes
Copy link
Contributor

@antonio-mendes antonio-mendes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 ! Will review the others and then merge all

models/arbitrum/arbitrum/arbitrum_arbitrum_airdrop_claims.sql
{% endif %}
{% if is_incremental() %}
WHERE t.evt_block_time >= date_trunc("day", now() - interval '1 week')
WHERE t.evt_block_time >= date_trunc('day', now() - interval '7' Day)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The airdrop is only live for the next ~20 days so let's try to remember and remove the incremental run then - Source: https://arbitrum.foundation/

antonio-mendes
antonio-mendes approved these changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@antonio-mendes antonio-mendes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 ! Will review other airdrop migrations and then batch merge them

@antonio-mendes antonio-mendes merged commit 505ef15 into duneanalytics:main Sep 6, 2023
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 6, 2023
@henrystats henrystats deleted the airdrop-claims-arbitrum branch September 8, 2023 21:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@antonio-mendes antonio-mendes antonio-mendes approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@henrystats @dune-eng @antonio-mendes