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

RANCHER-1334 Eureka related images into eureka-platform.json file for further Rancher deployments #154

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

eldiiar-duishenaliev
Copy link
Contributor

@eldiiar-duishenaliev eldiiar-duishenaliev commented Jul 30, 2024

@dcrossleyau
Copy link
Contributor

In the future please do not include code formatting changes in the same pull-request. Doing such obscures the real changes.

@dcrossleyau
Copy link
Contributor

Please refer to the Jira ticket if possible.

@dcrossleyau
Copy link
Contributor

What does "EPC" mean in the commit message?

@dcrossleyau
Copy link
Contributor

Thanks for explaining that this was tested.

@eldiiar-duishenaliev
Copy link
Contributor Author

What does "EPC" mean in the commit message?

Hello Sir,
EPC is acronym for Eureka Platform Complete. Sorry for this misunderstanding. I should provide this info beforehand.

@eldiiar-duishenaliev
Copy link
Contributor Author

Please refer to the Jira ticket if possible.

This activity is a part of story: https://folio-org.atlassian.net/browse/RANCHER-1334

@eldiiar-duishenaliev
Copy link
Contributor Author

In the future please do not include code formatting changes in the same pull-request. Doing such obscures the real changes.

Sure Sir, sorry, that's a bad habit to make formatting in whole file.

@dcrossleyau dcrossleyau changed the title Eureka related images into eureka-platform.json file for further Rancher deployments RANCHER-1334 Eureka related images into eureka-platform.json file for further Rancher deployments Jul 31, 2024
@dcrossleyau
Copy link
Contributor

Thanks for explaining. No need for Sir.

Sure, do cleanup -- just not in the same PR for review please. It would be easy for people to overlook something important.

Copy link
Contributor

@wafschneider wafschneider left a comment

Choose a reason for hiding this comment

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

In general, this looks OK to me. If I understand it correctly, the eureka.json file in the snapshot branch of platform-complete will be updated for every release and every commit to the default branch if the module is already in the eureka.json file.

This is not my code, so I'd like @funkymalc or @dcrossleyau to look at this before approval.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please confirm that there are only formatting changes in this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a one-line relevant change in each of the Docker-related files to call the updateEurekaFile script.

@wafschneider
Copy link
Contributor

Oops, sorry, @dcrossleyau, I didn't realize you had already reviewed, and had already addressed my main concern (formatting changes obscuring the actual code changes). Go forth!

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