Skip to content

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

Merged
shawncal merged 13 commits into
microsoft:mainfrom
rogerbarreto:features/import-multiple-skills
Apr 26, 2023
Merged

Adding Multiple Skill Directories Simplification + Integration Tests readme changes#599
shawncal merged 13 commits into
microsoft:mainfrom
rogerbarreto:features/import-multiple-skills

Conversation

@rogerbarreto

Copy link
Copy Markdown
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
Comment thread dotnet/src/SemanticKernel/KernelExtensions/ImportSemanticSkillFromDirectory.cs Outdated

@shawncal shawncal left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread samples/dotnet/kernel-syntax-examples/Example12_Planning.cs
@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
Copy Markdown
Contributor

@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
@rogerbarreto rogerbarreto dismissed stale reviews from shawncal and lemillermicrosoft via 109efdc April 26, 2023 07:16
@shawncal shawncal merged commit 5b8a269 into microsoft:main Apr 26, 2023
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.

3 participants