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

Adding Multiple Skill Directories Simplification + Integration Tests readme changes #599

Merged
merged 13 commits into from Apr 26, 2023

Conversation

RogerBarreto
Copy link
Member

Motivation and Context - Improvement

It is a common scenario having to import multiple skills.

Adding this as a param array to the extension will reduce number of calls and improve cleanliness.

Description

As part of this PR I also spotted missing in the Readme the recommendation on using UserSecrets for Integration Tests.

Changed the readme and example config files to use davinci-003 models as our current standard for integration tests.

Contribution Checklist

@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel.core labels Apr 22, 2023
@RogerBarreto RogerBarreto changed the title Adding multiple skill directories + integration tests readme improvem… Adding Multiple Skill Directories Simplification + Integration Tests readme changes Apr 22, 2023
@RogerBarreto RogerBarreto added the PR: ready for review All feedback addressed, ready for reviews label Apr 22, 2023
@shawncal shawncal force-pushed the features/import-multiple-skills branch from 1068129 to 4cf8ebd Compare April 23, 2023 22:10
Copy link
Member

@shawncal shawncal left a comment

Choose a reason for hiding this comment

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

Changes requested. Follow up with shawncal if needed.

@lemillermicrosoft lemillermicrosoft enabled auto-merge (squash) April 25, 2023 19:07
@lemillermicrosoft lemillermicrosoft added the PR: ready to merge PR has been approved by all reviewers, and is ready to merge. label Apr 25, 2023
@RogerBarreto RogerBarreto removed the request for review from shawncal April 25, 2023 19:48
shawncal
shawncal previously approved these changes Apr 26, 2023
@shawncal
Copy link
Member

@RogerBarreto Looks like we've got a build break after merge. Can you take a look please? Once that's resolved, should be good to go.

@shawncal shawncal disabled auto-merge April 26, 2023 07:13
@shawncal shawncal merged commit 5b8a269 into microsoft:main Apr 26, 2023
11 checks passed
dluc pushed a commit that referenced this pull request Apr 29, 2023
…readme changes (#599)

### Motivation and Context - Improvement
It is a common scenario having to import multiple skills. 
Adding this as a `param` array to the extension will reduce number of calls and improve cleanliness.

### Description
As part of this PR I also spotted missing in the Readme the recommendation on using UserSecrets for Integration Tests.
Changed the readme and example config files to use davinci-003 models as our current standard for integration tests.
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
…readme changes (microsoft#599)

### Motivation and Context - Improvement
It is a common scenario having to import multiple skills. 
Adding this as a `param` array to the extension will reduce number of calls and improve cleanliness.

### Description
As part of this PR I also spotted missing in the Readme the recommendation on using UserSecrets for Integration Tests.
Changed the readme and example config files to use davinci-003 models as our current standard for integration tests.
@RogerBarreto RogerBarreto deleted the features/import-multiple-skills branch December 5, 2023 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.NET Issue or Pull requests regarding .NET code PR: ready for review All feedback addressed, ready for reviews PR: ready to merge PR has been approved by all reviewers, and is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants