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

Export some CLI functionality to an API] #482

Merged

Conversation

@uzust01
Copy link
Contributor

commented Jul 12, 2019

No changes in commands or options.

Export some CLI loops to an recursive non-system API, add unit tests …
…for the new API

Signed-off-by: Uzunov <stanko.uzunov@broadcom.com>

@uzust01 uzust01 changed the title Export some CLI loops to an recursive non-system API, add unit tests … Export some CLI loops to an recursive non-system API] Jul 12, 2019

@uzust01 uzust01 changed the title Export some CLI loops to an recursive non-system API] Export some CLI functionality to an API] Jul 12, 2019

Remove unused variable
Signed-off-by: Uzunov <stanko.uzunov@broadcom.com>

expectZosmfResponseSucceeded(response, error);
expect(response).toEqual(FLATTEN_STEP);

This comment has been minimized.

Copy link
@tucker01

tucker01 Jul 12, 2019

Contributor

Totally up to you, but you could expect to match a snapshot. This way you wouldn't need to maintain the validation structure.

This comment has been minimized.

Copy link
@uzust01

uzust01 Jul 12, 2019

Author Contributor

Thanks @tucker01 , I have been having some issues with the build (I know why they are but I will not get into details here) so I decided to make it like this.

} else if(step.isRestStep) {
miscValue = `HTTP ${step.actualStatusCode}`;
}
const stepSummary: IStepSummary = {

This comment has been minimized.

Copy link
@tucker01

tucker01 Jul 12, 2019

Contributor

Are you dropping any of the fields from step when creating stepSummary? You could use the spread operator here and append the misc property.

@tucker01
Copy link
Contributor

left a comment

I like that we're moving useful functionality into the exported APIs. Good work. Left a couple comments, your choice to address.

@tucker01 tucker01 merged commit 5a3a738 into zowe:lts-incremental Jul 17, 2019

2 checks passed

DCO DCO
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
@zFernand0

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2019

@uzust01, @tucker01,
Should this be ported to master?

@tucker01

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2019

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.