Cleanup Adapter structure to not be used to pass parameters #5918
Conversation
|
@feloy: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
✅ Deploy Preview for odo-docusaurus-preview ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
3f16462 to
3e51559
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rm3l The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold Will need to be rebased after #5914 is merged (and not the other way) |
f6c7d54 to
e73b4ce
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| // RandomPorts is true to forward containers ports on local random ports | ||
| RandomPorts bool | ||
| // ErrOut is a Writer to output forwarded port information | ||
| ErrOut io.Writer |
There was a problem hiding this comment.
@feloy I remember us discussing this struct and various other structs used for successful execution of odo dev, and you mentioned that it's better to pass arguments than have fat structs. I'm trying to understand why you prefer adding more fields to a struct than using arguments.
NOTE: I'm not questioning your judgement. I'm only trying to understand your thought process in doing so.
There was a problem hiding this comment.
I preferred not to change this part for the moment.
To be more precise, I think that we should use structures for optional parameters only, and have parameters for mandatory parameters, so the compiler double-checks that we don't forget to pass mandatory values (function parameters) for us, what is not done with fields of structures (I have been trapped by this several times, not later than yesterday, see #5918 (comment))
|
/override ci/prow/unit |
|
@dharmit: Overrode contexts on behalf of dharmit: ci/prow/unit, ci/prow/v4.10-integration-e2e In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/override ci/prow/unit-and-validate-test |
|
@feloy: Overrode contexts on behalf of feloy: ci/prow/unit-and-validate-test In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
…eveloper#5918) * Do not use receiver to pass DevfileCommands parameters * Do not use receiver to pass DebugPort parameter * Do not use receiver to pass deployment parameter * Do not use receiver to pass pod parameter * Move randomPorts and errOut as PushParameters
What type of PR is this:
/kind cleanup
What does this PR do / why we need it:
See more details on commits messages
Which issue(s) this PR fixes:
Fixes partly #5867
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer: