-
Notifications
You must be signed in to change notification settings - Fork 79
[KOGITO-3568] - Remove spec.httpPort attribute from Operator and HTTP… #630
Conversation
/jenkins test |
Codecov Report
@@ Coverage Diff @@
## master #630 +/- ##
==========================================
- Coverage 42.01% 37.70% -4.31%
==========================================
Files 169 146 -23
Lines 9012 6224 -2788
==========================================
- Hits 3786 2347 -1439
+ Misses 4812 3502 -1310
+ Partials 414 375 -39
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/jenkins test |
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.
There is a BDD scenario to cover the functionality that has been removed: update_httpport.feature. Therefore, this feature needs to be removed as well.
Change detected in the PR, requesting reviews and running pipeline(if required) again |
Thanks, removed the file :) |
/jenkins test |
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.
I like to see the old code going away. Nice job! You've been doing a great job removing things from the operator. :)
@Kaitou786 please update the RELEASE_NOTES.md file. Maybe we should add this rule to the bot? |
Change detected in the PR, requesting reviews and running pipeline(if required) again |
3c96464
to
b671bab
Compare
Change detected in the PR, requesting reviews and running pipeline(if required) again |
Done, for the bot what kind of rule do we want, every PR which have a milestone should be added to RELEASE_NOTES.md? |
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.
Remove HTTPPort
from following locations :
-
Remove
setHTTPPortInEnvVar
function. No more required.
https://github.com/Kaitou786/kogito-cloud-operator/blob/0576a63b824a4e86fe24733951c3879ce225e6e1/pkg/infrastructure/services/deployment.go#L90 -
No need to pass HTTPPort in function args. We can directly initialize it with
framework.DefaultExposedPort
https://github.com/Kaitou786/kogito-cloud-operator/blob/154a2d0a1c7af1ab3eabe894d03016268815551f/pkg/infrastructure/services/probe.go#L95 -
Also need to remove
HTTPPort
from kogito-images as well
Change detected in the PR, requesting reviews and running pipeline(if required) again |
1 similar comment
Change detected in the PR, requesting reviews and running pipeline(if required) again |
@vaibhavjainwiz Thanks for noticing these :), I added all your comments can you take a second look :) |
/jenkins test |
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.
LGTM
/jenkins test |
/jenkins test |
@sutaakar do you want to take a look before I merge this one? |
…_PORT env var from Kogito images See: https://issues.redhat.com/browse/KOGITO-3568 Signed-off-by: Tarun Khandelwal <tarkhand@redhat.com>
d52b016
to
ae82e22
Compare
Change detected in the PR, requesting reviews and running pipeline(if required) again |
Change detected in the PR, requesting reviews and running pipeline(if required) again |
/jenkins test |
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.
Looks good to me
…_PORT env var from Kogito images
See: https://issues.redhat.com/browse/KOGITO-3568
Signed-off-by: Tarun Khandelwal tarkhand@redhat.com
Many thanks for submiting your Pull Request ❤️!
Please make sure that your PR meets the following requirements:
[KOGITO-XYZ] Subject