Skip to content
This repository has been archived by the owner on Dec 15, 2023. It is now read-only.

Move splitted operation references to cli-split-operation-splitted-operations #66

Merged
merged 3 commits into from
Jun 9, 2020

Conversation

shawncx
Copy link
Contributor

@shawncx shawncx commented Jun 8, 2020

Put splitted operation references in cli-operations cause confusing. This pr move those references into independent extension property cli-split-operation-splitted-operations

@shawncx shawncx requested review from RodgeFu and qiaozha June 8, 2020 03:02
@@ -219,6 +220,16 @@ export class NodeHelper {
return NodeHelper.getExtensionsProperty(op, NodeHelper.SPLIT_OPERATION_ORIGINAL_OPERATION, null);
}

public static addSplitOperationSplittedOperations(op: Operation, splittedOp: Operation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this name is confusing, will you be able to understand it 3 month later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Thanks

src/helper.ts Outdated
@@ -347,6 +347,7 @@ export class Helper {
(isNullOrUndefined(NodeHelper.getPolyAsResourceParam(vv)) ? '' : `${NEW_LINE}${tab(3)}cli-poly-as-resource-subclass-param: ${NodeHelper.getCliKey(NodeHelper.getPolyAsResourceParam(vv), '<missing-clikey>')}`) +
(isNullOrUndefined(NodeHelper.getPolyAsResourceOriginalOperation(vv)) ? '' : `${NEW_LINE}${tab(3)}cli-poly-as-resource-original-operation: ${NodeHelper.getCliKey(NodeHelper.getPolyAsResourceOriginalOperation(vv), '<missing-clikey>')}`) +
(isNullOrUndefined(NodeHelper.getSplitOperationOriginalOperation(vv)) ? '' : `${NEW_LINE}${tab(3)}cli-split-operation-original-operation: ${NodeHelper.getCliKey(NodeHelper.getSplitOperationOriginalOperation(vv), '<missing-clikey>')}`) +
(NodeHelper.getSplitOperationSplittedOperations(vv, () => []).length === 0 ? '' : `${NEW_LINE}${tab(3)}cli-split-operation-splitted-operations: ${NodeHelper.getSplitOperationSplittedOperations(vv, () => []).map((vvv) => NodeHelper.getCliKey(vvv, '<missing-clikey>')).join(',')}`) +
`${NEW_LINE}${tab(3)}parameters:${NEW_LINE}`.concat(
vv.parameters.map(vvv => `${tab(3)}- parameterName: ${generateCliValue(vvv, 4)}${generatePropertyFlattenValue(vvv, 4)}${generatePropertyReadonlyValue(vvv, 4)}${generateDiscriminatorValueForParam(vvv, 4)}${NEW_LINE}` +
(isNullOrUndefined(NodeHelper.getPolyAsResourceBaseSchema(vvv)) ? '' : `${tab(4)}cli-poly-as-resource-base-schema: ${NodeHelper.getCliKey(NodeHelper.getPolyAsResourceBaseSchema(vvv), '<baseSchemaCliKeyMissing>')}${NEW_LINE}`) +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code is getter more and more complex, feel free to refactor it if you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I will do refactor later, thanks

@@ -219,6 +220,16 @@ export class NodeHelper {
return NodeHelper.getExtensionsProperty(op, NodeHelper.SPLIT_OPERATION_ORIGINAL_OPERATION, null);
}

public static addSplitOperationIntoOperations(op: Operation, splittedOp: Operation) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about make it more generic to have it in NodeHelper, we dont need split concept here, for example, addExtendedOperation(node: Operation, operation: Operation)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am worried about generic may cause confusing. Because previously we put these operations in cli-operations, which makes az cannot know these operations come from poly or split. That's why I create an independent property for split.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed reference. Thanks

@qiaozha
Copy link
Member

qiaozha commented Jun 8, 2020

This works from my side now

@shawncx
Copy link
Contributor Author

shawncx commented Jun 8, 2020

@qiaozha thanks for confirming. But we decide to delete the reference from original operation to splitted operations. Because we regard it is unnecessary. We can use the cli-split-operation-original-operation on splitted operations to trace the original operation.

@shawncx shawncx requested a review from RodgeFu June 8, 2020 11:37
@shawncx shawncx merged commit c036931 into Azure:master Jun 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants