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

Update service discovery references #872

Merged
merged 8 commits into from
Sep 8, 2021
Merged

Update service discovery references #872

merged 8 commits into from
Sep 8, 2021

Conversation

ersonp
Copy link
Contributor

@ersonp ersonp commented Sep 7, 2021

Did you run make format && make check?

Fixes #

Changes:

  • Change proxyDiscovery to serviceDiscovery

How to test this PR:

Copy link
Member

@jdknives jdknives left a comment

Choose a reason for hiding this comment

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

Should we rename the package appdisc as well? Or is just responsible for updating app statuses in service discovery? Also do we need to define a config migration, right ? Also, please add changes to changelog. Will need to start doing that more diligently.

pkg/visor/visorconfig/v1.go Outdated Show resolved Hide resolved
@ersonp
Copy link
Contributor Author

ersonp commented Sep 7, 2021

  • We can name appdisc to updatedisc instead since that is what it's doing since appdisc can be confusing.
  • The updateUrls in parse takes care of the migration. Only thing is that the URL is changed to http://sd.skycoin.com with it.
  • Will add all the changes of config v1.1.0 to Changelog. Also, I will need some inputs for all the changes introduced in the current version of skywire@develop.

@ersonp ersonp changed the title Update sd Update service discovery refrences Sep 8, 2021
@ersonp ersonp changed the title Update service discovery refrences Update service discovery references Sep 8, 2021
@jdknives jdknives merged commit 89b0fca into skycoin:develop Sep 8, 2021
@ersonp ersonp deleted the update-sd branch April 11, 2022 15:38
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