-
Notifications
You must be signed in to change notification settings - Fork 83
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
Add Builder Codes #66
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.
Change looks good. Mostly style nits
This is marked as a draft. Did you test/is it ready to be merged? |
I'm in the process of adding the |
Hey @lmlmt, the code is ready for review and to be merged |
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.
Mostly looks good, I don't think a separate BulkOrderWithBuilder
is needed though
@Frixoe Can you make sure it compiles? The continuous integration is now failing so would be good to make sure that's in a good spot before merging. |
@lmlmt sorry completely forget about my arch nemesis clippy |
I've added the initial builder codes approval code having referenced it from the python sdk and I think I'm missing something in the implementation related to my understanding of the builders code API.
When I tried running it initially I got a 422 and now I get a 403 but that's probably a personal environment issue. But would be great to get some feedback on the current implementation. Please ignore the info! lines.