Skip to content

chore: minor changes in AddressBook to support planned move #18933

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

Merged
merged 1 commit into from
Apr 23, 2025

Conversation

netopyr
Copy link
Contributor

@netopyr netopyr commented Apr 22, 2025

Description:

This PR introduces some small changes that will make the planned move of Address and AddressBook to the consensus-model module simpler.

  • AddressBookIterator is now an inner class of AddressBook. This is a pretty common approach for collections. The iterator was not referenced anywhere else this avoids an internal package with a single class.
  • Removed AddressBook.toConfigText(). This introduced a dependency on outputting a table which does not belong in a model. The method forwarded to AddressBookUtils anyway.

Related issue(s):

Part of #18927

@netopyr netopyr added this to the v0.62 milestone Apr 22, 2025
@netopyr netopyr self-assigned this Apr 22, 2025
@netopyr netopyr requested a review from a team as a code owner April 22, 2025 12:43
@netopyr netopyr modified the milestones: v0.62, v0.63 Apr 22, 2025
@lpetrovic05
Copy link
Contributor

@netopyr should we focus on removing address book rather than moving it?

@netopyr
Copy link
Contributor Author

netopyr commented Apr 22, 2025

@netopyr should we focus on removing address book rather than moving it?

No. Removing the address book is a huge task that will take quite some time while blocking modularization. Moving the address book to a different module can be done quickly.

@netopyr netopyr merged commit 107e8c5 into main Apr 23, 2025
45 checks passed
@netopyr netopyr deleted the 18927-prepare-addressbook branch April 23, 2025 11:01
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