-
Notifications
You must be signed in to change notification settings - Fork 61
chore: create permit table and move data #817
base: development
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ubiquibot-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@pavlovcik PR attached |
From #787 Shouldn't you refactor functions that interact with these tables? |
Yea, I would do that now.. |
check it out now, i ran into some more issues and fixed.. check description |
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.
Code looks good just have some rename requests
Fixed issues |
Ping @pavlovcik @whilefoo |
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.
I'd rather not use char(x) because we've had some problems in the past related to that because either Supabase or PostgREST library wasn't working correctly, so I would just use TEXT with CHECK (char_length(name) <= x)
Even if we ignore the bug with char(x), people are mostly recommending to use text because char and varchar have some limits
@pavlovcik what's your thought on this, and did the schema change again? I'm seeing some changes on the parent issue |
To be honest I spent a significant amount of time trying to completely redo the database and functions that interact with it. I seem to have underestimated the amount of work involved but it forces me to become intimately familiar with all of these technologies and concepts within our bot. It might be more productive to focus efforts elsewhere this week as I continue my research |
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 EVM address we're currently using has a fixed size 42
. so no need to set varchar(42)
type for them
@0xcodercrane @whilefoo i made it a |
@seprintour can you make new QA issue to make sure everything is working? |
Sure |
Resolves #811
Quality Assurance:
More QA: Seprintour-Test/test#41 (comment)
I also changed the datatype from
char
tovarchar
to fix this error Seprintour-Test/test#41 (comment) @pavlovcik