-
Notifications
You must be signed in to change notification settings - Fork 57
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
Feature/svc common usage (Jenkins / Nexus / GitService) #325
Feature/svc common usage (Jenkins / Nexus / GitService) #325
Conversation
@martsec @georgfedermann - fyi, the only thing that changed are packages ... |
@@ -296,4 +296,11 @@ class OpenShiftService { | |||
return images | |||
} | |||
|
|||
String getOriginUrlFromBuildConfig (String project, String bcName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could still use the instance variable project
, right? I know that how it is used from the pipeline, here the project
is e.g. foo-cd
and for the other methods it is e.g. foo-dev
, but that shouldn't matter. You anyway instantiate a new OpenShiftService
just to call this method, so you can pass the project to the constructor there ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, I can also do this cleanup later on - I need to work on the OpenShiftService this week anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michaelsauter - I would leave this to you .. to do this in one big shot ..
This PR also ensures that common services are really shared at runtime (and not blindly overwritten in the 'component' pipeline) .. which uncovered a whole set of more issues (Pipelinesteps, NPE,.. :))
Tested both standalone (with flask and also in the MRO context) - both work now :)