Skip to content

Move vat_fold & index it by ilk#49

Merged
m0ar merged 2 commits intostagingfrom
vdb-1062-fix-vat-fold
Dec 10, 2019
Merged

Move vat_fold & index it by ilk#49
m0ar merged 2 commits intostagingfrom
vdb-1062-fix-vat-fold

Conversation

@m0ar
Copy link
Copy Markdown
Contributor

@m0ar m0ar commented Dec 10, 2019

No description provided.

@m0ar m0ar requested a review from rmulhol December 10, 2019 13:09
@m0ar m0ar self-assigned this Dec 10, 2019
Copy link
Copy Markdown
Contributor

@rmulhol rmulhol left a comment

Choose a reason for hiding this comment

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

Thanks for knocking this out!

Seems like a lot of these files would benefit from goimports -w. One annoyance with that is that it seems like that will always insert a line break between core utils and other deps, but doesn't uniformly assemble all the other deps together without line breaks (which requires deliberately removing the breaks before running goimports)

log_id BIGINT NOT NULL REFERENCES header_sync_logs (id) ON DELETE CASCADE,
urn_id INTEGER NOT NULL REFERENCES maker.urns (id) ON DELETE CASCADE,
ilk_id INTEGER NOT NULL REFERENCES maker.ilks (id) ON DELETE CASCADE,
u TEXT NOT NULL,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should u potentially be an int referencing public.addresses (id)? Not sure if it matters if this is always just the address for the vow, but thinking that would match our current approach elsewhere

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Didn't see that for vat_suck which is very similar, that's why I didn't :)

Copy link
Copy Markdown
Contributor

@rmulhol rmulhol Dec 10, 2019

Choose a reason for hiding this comment

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

🤔 seems like maybe we should make the change in both places? just thinking that the addresses table should be a reliable source for every known address for the contracts/events we track

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm maybe that's true.. Would you mind creating tasks for this? I'm quite sure there are more ad-hoc addresses that we don't reference there that we should also catch in that cleanup.

@m0ar m0ar force-pushed the vdb-1062-fix-vat-fold branch from c187c57 to 45c6ad7 Compare December 10, 2019 17:40
@m0ar m0ar merged commit 27b64b5 into staging Dec 10, 2019
@m0ar m0ar deleted the vdb-1062-fix-vat-fold branch December 10, 2019 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants