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
Remove deadcode from test helpers. #4647
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4647 +/- ##
==========================================
- Coverage 84.44% 78.78% -5.66%
==========================================
Files 183 183
Lines 8377 8377
Branches 566 566
==========================================
- Hits 7074 6600 -474
- Misses 1303 1777 +474
Continue to review full report at Codecov.
|
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.
Checked. LGTM
@rabbah we have found time to adapt the affected test cases. Thanks for waiting with the merge |
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
* | ||
* @return VCAP_SERVICES as a JSON object | ||
*/ | ||
public static JsonObject getVCAPServices() { |
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.
@rabbah
Unfortunately there are tests specific to IBM Functions using getVCAPServices
(and with it vcapFile
).
Leaving these two things in for now would give our team time to adapt changes without direct pressure.
getVCAPcredentials
, getCredentials
and DEFAULT_CONCURRENCY
are not used.
If there is agreement I would like ask to remove the later ones only and get to the used ones in a later PR.
this needs to be reverted. there are openwhisk repos that use the removed utils. openwhisk is made up of many repos/projects and you cannot remove utils just because you do not see them being used in this repo/project. |
Thanks @dubee for pointing that out. I didn't mark this PR ready for review or notify the dev list because I did in fact intend to move the related code to the providers. I'm aware the providers need them (see the corresponding issue from July 2018). I'm not sure why this was merged yet (the message from Martin did suggest time was needed to address dependencies). |
FWIW, It's OK with me to revert the PR. I can reintroduce the patch when the downstream dependencies are resolved. |
Removes code that appears dead in this project/tests.
Closes #3863.
Description
Related issue and scope
My changes affect the following components
Types of changes
Checklist: