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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed missing today() in TimeSkill #577

Merged
merged 8 commits into from
Apr 24, 2023

Conversation

BeanHsiang
Copy link
Contributor

Motivation and Context

the method Today() in TimeSkill.cs and today() in TimeSkill.py was missing.
Today must be an alias for Date.

Description

Add the method Today.

Contribution Checklist

@MovGP0
Copy link

MovGP0 commented Apr 21, 2023

Instead of

    [SKFunction("Get the current date")]
    public string Today()
    {
        // Example: Sunday, 12 January, 2025
        return this.Date();
    }

You could just write

    [SKFunction("Get the current date")]
    public string Today() => Date();

@BeanHsiang
Copy link
Contributor Author

that's better one.

@lemillermicrosoft lemillermicrosoft added PR: feedback to address Waiting for PR owner to address comments/questions python Pull requests for the Python Semantic Kernel kernel Issues or pull requests impacting the core kernel labels Apr 21, 2023
@BeanHsiang
Copy link
Contributor Author

@lemillermicrosoft
no change in any .md file, please help to pass markdown-link-check

@awharrison-28 awharrison-28 added PR: ready to merge PR has been approved by all reviewers, and is ready to merge. and removed PR: feedback to address Waiting for PR owner to address comments/questions labels Apr 22, 2023
@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel.core and removed kernel Issues or pull requests impacting the core kernel labels Apr 24, 2023
@lemillermicrosoft lemillermicrosoft enabled auto-merge (squash) April 24, 2023 19:39
Copy link
Member

@lemillermicrosoft lemillermicrosoft left a comment

Choose a reason for hiding this comment

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

I understand the concern of duplicate functions having some cost elsewhere. But I think that's something we should solve elsewhere as this will happen in the wild, too.

@lemillermicrosoft lemillermicrosoft merged commit e207b37 into microsoft:main Apr 24, 2023
22 checks passed
dluc pushed a commit that referenced this pull request Apr 29, 2023
### Motivation and Context
the method Today() in TimeSkill.cs and today() in TimeSkill.py was
missing.
Today must be an alias for Date.

### Description
Add the method Today.

Co-authored-by: Abby Harrison <54643756+awharrison-28@users.noreply.github.com>
dehoward pushed a commit to lemillermicrosoft/semantic-kernel that referenced this pull request Jun 1, 2023
### Motivation and Context
the method Today() in TimeSkill.cs and today() in TimeSkill.py was
missing.
Today must be an alias for Date.

### Description
Add the method Today.

Co-authored-by: Abby Harrison <54643756+awharrison-28@users.noreply.github.com>
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 to merge PR has been approved by all reviewers, and is ready to merge. python Pull requests for the Python Semantic Kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants