Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Comments

Support merging .uischema files#852

Merged
chrimc62 merged 33 commits intomasterfrom
chrimc/pkg
Jul 15, 2020
Merged

Support merging .uischema files#852
chrimc62 merged 33 commits intomasterfrom
chrimc/pkg

Conversation

@chrimc62
Copy link

@chrimc62 chrimc62 commented Jun 11, 2020

This adds to dialog:merge the ability to merge .uischema files to implement #852 .
To do this it walks-over the component (.csproj, package.json, .nuspec, nuget, packages.config) tree and builds a breadth-first parent before child order to use for when merging .uischema definitions. The uischema files are checked to ensure they are valid according to the uischema definition and to ensure that all properties are found in the schema. The result is a merged ui schema file per locale like [.locale].uischema.

It will also do an analysis and show errors in content files of:

  1. Multiple of the same asset like (.lu, .lg, .qna, .schema, .uischema, .dialog) within a component.
  2. Different components that define the same .schema file.

Also added the ability to infer the output file name from the first project.

NOTE: I did keep the ability to do .schema files directly as patterns. This is mainly for testing.

@chrimc62 chrimc62 requested a review from tomlm June 11, 2020 05:49
@chrimc62 chrimc62 requested a review from munozemilio June 11, 2020 07:05
@chrimc62 chrimc62 marked this pull request as ready for review June 11, 2020 18:13
@chrimc62 chrimc62 requested a review from cleemullins as a code owner June 11, 2020 18:13
Copy link
Contributor

@tomlm tomlm left a comment

Choose a reason for hiding this comment

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

:shipit:

@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2020

Codecov Report

Merging #852 into master will increase coverage by 0.78%.
The diff coverage is 83.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #852      +/-   ##
==========================================
+ Coverage   57.90%   58.69%   +0.78%     
==========================================
  Files         218      217       -1     
  Lines       15364    15619     +255     
  Branches     2060     2131      +71     
==========================================
+ Hits         8897     9167     +270     
+ Misses       5919     5884      -35     
- Partials      548      568      +20     
Impacted Files Coverage Δ
packages/dialog/src/commands/dialog/merge.ts 0.00% <0.00%> (ø)
packages/dialog/src/library/schemaMerger.ts 79.40% <84.65%> (+5.10%) ⬆️
...kages/luis/src/commands/luis/application/create.ts 60.00% <0.00%> (-24.22%) ⬇️
...mathematics/confusion_matrix/AppConfusionMatrix.ts 6.66% <0.00%> (-22.32%) ⬇️
...rc/commands/luis/application/assignazureaccount.ts 80.00% <0.00%> (-20.00%) ⬇️
packages/lg/src/commands/lg/index.ts 66.66% <0.00%> (-16.67%) ⬇️
packages/qnamaker/src/commands/qnamaker/index.ts 66.66% <0.00%> (-16.67%) ⬇️
...ages/luis/src/commands/luis/application/publish.ts 85.71% <0.00%> (-14.29%) ⬇️
packages/luis/src/commands/luis/train/show.ts 89.47% <0.00%> (-10.53%) ⬇️
...ackages/luis/src/commands/luis/application/show.ts 89.47% <0.00%> (-10.53%) ⬇️
... and 65 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59f950d...000655b. Read the comment docs.

@chrimc62 chrimc62 changed the title Switch to override model for .schema and composer asset files Support merging .uischema files Jul 14, 2020
@chrimc62 chrimc62 requested a review from a-b-r-o-w-n July 14, 2020 19:14
* [`bf dialog`](#bf-dialog)
* [`bf dialog:merge PATTERNS`](#bf-dialogmerge-patterns)
* [`bf dialog:verify PATTERNS`](#bf-dialogverify-patterns)
- [@microsoft/bf-dialog](#microsoftbf-dialog)
Copy link
Contributor

@munozemilio munozemilio Jul 14, 2020

Choose a reason for hiding this comment

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

This should be outside the commands toc otherwise it will be deleted on the next documentation autogen

Copy link
Author

Choose a reason for hiding this comment

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

I did auto-gen and then restored things from the original. For example the slashes for code references are still the wrong way.

Copy link
Contributor

@munozemilio munozemilio left a comment

Choose a reason for hiding this comment

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

Some details you may want to take care before merge

```

_See code: [src/commands/dialog/index.ts](https://github.com/microsoft/botframework-cli/tree/master/packages/dialog/src/commands/dialog/index.ts)_
_See code: [src/commands/dialog/index.ts](https://github.com/microsoft/botframework-cli/tree/master/packages/dialog/blob/v1.0.0/src/commands/dialog/index.ts)_
Copy link
Contributor

Choose a reason for hiding this comment

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

Url wont work

```

_See code: [src/commands/dialog/merge.ts](https://github.com/microsoft/botframework-cli/tree/master/packages/dialog/src/commands/dialog/merge.ts)_
_See code: [src/commands/dialog/merge.ts](https://github.com/microsoft/botframework-cli/tree/master/packages/dialog/blob/v1.0.0/src/commands/dialog/merge.ts)_
Copy link
Contributor

Choose a reason for hiding this comment

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

url won t work

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