Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Moving networking related parameters off of the node config #3394

Merged
merged 1 commit into from
Jul 26, 2021
Merged

Moving networking related parameters off of the node config #3394

merged 1 commit into from
Jul 26, 2021

Conversation

clemahieu
Copy link
Contributor

Moving networking related parameters off of the node config class an on to the networking config class where they belong.

@clemahieu
Moving networking related parameters off of the node config class an …
012105f 
…on to the networking config class where they belong.
theohax
theohax approved these changes Jul 26, 2021
View reviewed changes
Copy link
Contributor

@theohax theohax left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Refactoring looks good (taking what is not node-config related off it), but if anything, I find the following access rather cumbersome from a semantic perspective:

node.network_params.network

Not a deal breaker, just thinking out loud.

@clemahieu clemahieu merged commit 32aa53c into develop Jul 26, 2021
@clemahieu clemahieu deleted the move_network_params branch July 26, 2021 15:23
@zhyatt
Copy link
Collaborator

zhyatt commented Jul 26, 2021

@clemahieu Was this all internal changes or are there some external toml file impacts to note?

@zhyatt zhyatt added the quality improvements This item indicates the need for or supplies changes that improve maintainability label Jul 26, 2021
@zhyatt zhyatt added this to the V23.0 milestone Jul 26, 2021
@zhyatt
Copy link
Collaborator

zhyatt commented Nov 23, 2021

@clemahieu Was this all internal changes or are there some external toml file impacts to note?

Colin confirmed no external configuration file impacts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@theohax theohax theohax approved these changes

Assignees
No one assigned
Labels
quality improvements This item indicates the need for or supplies changes that improve maintainability
Projects
None yet
Milestone
V23.0
Development

Successfully merging this pull request may close these issues.
3 participants
@clemahieu @zhyatt @theohax