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.

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

go/oasis-node/cmd/config/migrate: Add runtime P2P seed address too #5332

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

abukosek
Copy link
Contributor

When running the config migrate command, an additional P2P runtime seed address is added when migrating seed nodes.

@abukosek abukosek force-pushed the andrej/bugfix/migrate-seed-p2p-addrs branch from ffa35e3 to 9304789 Compare July 26, 2023 14:28
@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #5332 (9304789) into master (fbeae8d) will decrease coverage by 0.49%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

❗ Current head 9304789 differs from pull request most recent head ad143c1. Consider uploading reports for the commit ad143c1 to get more accurate results

@@            Coverage Diff             @@
##           master    #5332      +/-   ##
==========================================
- Coverage   67.64%   67.16%   -0.49%     
==========================================
  Files         525      525              
  Lines       55806    55812       +6     
==========================================
- Hits        37752    37488     -264     
- Misses      13534    13773     +239     
- Partials     4520     4551      +31     
Files Changed Coverage Δ
go/oasis-node/cmd/config/migrate/migrate.go 65.62% <100.00%> (+0.46%) ⬆️

... and 70 files with indirect coverage changes

@abukosek abukosek marked this pull request as ready for review July 26, 2023 15:33
Copy link
Contributor

@peternose peternose left a comment

Choose a reason for hiding this comment

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

Works! Approving, the comments can be taken into account or not.

Not sure if this is a bugfix, though 😄

go/oasis-node/cmd/config/migrate/migrate.go Outdated Show resolved Hide resolved
go/oasis-node/cmd/config/migrate/migrate.go Outdated Show resolved Hide resolved
.changelog/5332.bugfix.md Outdated Show resolved Hide resolved
@abukosek abukosek force-pushed the andrej/bugfix/migrate-seed-p2p-addrs branch from 9304789 to 9fe270e Compare July 27, 2023 12:50
@abukosek
Copy link
Contributor Author

Fixed :)

Copy link
Contributor

@peternose peternose left a comment

Choose a reason for hiding this comment

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

Looks great! 👍

@@ -134,6 +134,23 @@ func convertP2P(i interface{}, oldS string, newS string) []string {
return out
}

// For each Tendermint seed node address, append the corresponding
Copy link
Contributor

Choose a reason for hiding this comment

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

For the future, I believe that comments in Golang should start with the function's name. The same holds for variables. Unfortunately, I cannot find any useful links to support my claim 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll fix this comment :)

@abukosek abukosek force-pushed the andrej/bugfix/migrate-seed-p2p-addrs branch from 9fe270e to ad143c1 Compare July 27, 2023 13:13
@abukosek abukosek enabled auto-merge July 27, 2023 14:48
@abukosek abukosek merged commit 4425f68 into master Jul 27, 2023
1 check passed
@abukosek abukosek deleted the andrej/bugfix/migrate-seed-p2p-addrs branch July 27, 2023 15:18
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.

3 participants