-
Notifications
You must be signed in to change notification settings - Fork 177
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
The banishment of Address #1880
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.
lgtm
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.
Trusting kim, just in case I somehow wound up as a code owner on this. I have not actually reviewed this PR, but Kim's review satisfies me.
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 have not tested this but I've reviewed the files I'm a code-owner of. I left one follow-up note for Zeke but these changes look correct to me.
Although I am a partial author of this, I just want to so that this one looks good to me. |
Description of Changes
Jeff and I worked on Friday to remove all uses of
Address
as an identifier for a database.Address
is still used for what will eventually becomeConnectionId
.There is a corresponding private PR.
API and ABI breaking changes
This breaks the CLI API as well as the HTTP and WebSocket APIs. It also edits the system tables.
Expected complexity level and risk
4
This complexity rating applies not only to the complexity apparent in the diff,
but also to its interactions with existing and future code.
This impacts many APIs as mentioned above. The change is mostly mechanical, but not entirely.
Testing
Describe any testing you've done, and any testing you'd like your reviewers to do,
so that you're confident that all the changes work as expected!