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

Conversation

@pavolum
Copy link
Contributor

@pavolum pavolum commented Sep 15, 2020

Description

This is the first of a series of PRs for VA Creation in Composer. This PR covers the addition of the VA Core template to Composer behind an env variabe (VA_CREATION) feature flag. fixes #4142

To test, pull this branch, set the VA_CREATION env variable to true in the command line and run yarn start:dev in that same command line. This will result in 'va-core' appearing in the list of bot templates.

At current state the VA template is functional in Composer and can run locally if you have preprovisioned LUIS and QNA resources that you can publish to.

NOTE:

  • Most of this PR is comprised of the VA core template itself which is maintained here. I would not review this code (everything in the vacre/template folder) at this point and instead review the "glue" code that loads this template as a plug in in Composer. This template code will be maintained and reviewed in its own repo (linked above) and we will periodically update the template code in Composer to reflect the latest until we determine a more ideal long term solution.
  • Initially the VA template had a corresponding custom runtime associated with it. I have removed this from the PR and set it so the VA Template uses the Composer base runtime. The custom runtime discussion is still in the air but it seems like we are straying away from the concept and will instead want everything to work on a single base runtime. Until we decide otherwise I am removing the custom runtime code for the time being.

@pavolum pavolum changed the title Adding VA template behind feature flag in Composer feature: Adding VA template behind feature flag in Composer Sep 15, 2020
@pavolum pavolum linked an issue Sep 15, 2020 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Sep 17, 2020

Coverage Status

Coverage remained the same at 56.026% when pulling 55e3afe on pavolum/va-template into 9e98dfc on main.

GeoffCoxMSFT
GeoffCoxMSFT previously approved these changes Oct 6, 2020
Copy link
Member

@GeoffCoxMSFT GeoffCoxMSFT left a comment

Choose a reason for hiding this comment

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

Looks good to me, but you should have a more knowledgeable person with the plug-ins and templating approve before committing.

Copy link
Contributor

@benbrown benbrown left a comment

Choose a reason for hiding this comment

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

There are some extraneous files here that should probably be removed.

Also, duplicating the boilerplate content is probably not good - we should make sure we ONLY include files that are NOT including in the boilerplate or runtime folders, and make sure they merge properly during creation. (/scripts folder, /schemas folder)

@cwhitten cwhitten dismissed benbrown’s stale review October 12, 2020 16:46

Ben this will be addressed with the upcoming conversational-core deliverable

@cwhitten cwhitten merged commit 31ec081 into main Oct 12, 2020
@cwhitten cwhitten deleted the pavolum/va-template branch October 12, 2020 17:03
@cwhitten cwhitten mentioned this pull request Nov 13, 2020
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
…t#4140)

* Adding va core template plug in with corresponding plugin build steps

* Adding env variable feature flag around VA=Core template

* removing unused code that was initially added for custom runtime which has been removed

* Updating react version in creation extension

* Formatting template strings for localization

Co-authored-by: Patrick Volum <[email protected]>
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.

Add VA template behind feature flag in Composer

8 participants