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

feat: let the user decide if they want planning to fail if no services are detected #878

Merged

Conversation

HarikrishnanBalagopal
Copy link
Contributor

Signed-off-by: Harikrishnan Balagopal harikrishmenon@gmail.com

…s are detected

Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
@github-actions
Copy link

Thanks for making a pull request! 😃
One of the maintainers will review and advise on the next steps.

@github-actions github-actions bot added the feat label Sep 27, 2022
@codecov
Copy link

codecov bot commented Sep 27, 2022

Codecov Report

Base: 18.40% // Head: 18.40% // No change to project coverage 👍

Coverage data is based on head (3a0cf0b) compared to base (a8f55e3).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #878   +/-   ##
=======================================
  Coverage   18.40%   18.40%           
=======================================
  Files          46       46           
  Lines        3672     3672           
=======================================
  Hits          676      676           
  Misses       2831     2831           
  Partials      165      165           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@anwarchk
Copy link

If I understand it correctly, move2kube doesn't do anything with the bound services present in the cf manifest. So is this change from a transformers perspective ?

@ashokponkumar
Copy link
Member

If I understand it correctly, move2kube doesn't do anything with the bound services present in the cf manifest. So is this change from a transformers perspective ?

@anwarchk This particular PR has nothing to do with Cf bound services. But we are currently working on a capability to migrate bound services to either as Operators or services in Cloud catalogs. Here is a discussion on this topic.

Please do let us know your usecase, how you are using Move2Kube, and how you would like Move2Kube transform the services, so that we can feed that into feature design for services migration. We can also continue the conversation in slack.

cmd/plan.go Outdated
logrus.Debugf("Plan : %+v", p)
logrus.Fatalf("failed to find any services or default transformers.")
if flags.failOnEmptyPlan {
logrus.Fatalf("Did not detect any services in the directory %s", srcpath)
Copy link
Member

Choose a reason for hiding this comment

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

we might also want to mention about default transformers similar to the previous message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

…services are detected

Signed-off-by: Harikrishnan Balagopal <harikrishmenon@gmail.com>
Comment on lines +147 to +149
logrus.Fatalf("Did not detect any services in the directory %s . Also we didn't find any default transformers to run.", srcpath)
}
logrus.Warnf("Did not detect any services in the directory %s . Also we didn't find any default transformers to run.", srcpath)
Copy link
Member

Choose a reason for hiding this comment

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

Should we say InvokedByDefault transformers to stick with a single terminology

@ashokponkumar ashokponkumar merged commit ccc9250 into konveyor:main Sep 28, 2022
@HarikrishnanBalagopal HarikrishnanBalagopal deleted the feat/optionalfailing branch September 28, 2022 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants