Skip to content

Python: Pysdk issue 3980#5063

Merged
moonbox3 merged 6 commits intomicrosoft:mainfrom
rewrlution:pysdk-issue-3980
Feb 22, 2024
Merged

Python: Pysdk issue 3980#5063
moonbox3 merged 6 commits intomicrosoft:mainfrom
rewrlution:pysdk-issue-3980

Conversation

@rewrlution
Copy link
Contributor

@rewrlution rewrlution commented Feb 18, 2024 •
edited
Loading

Motivation and Context

This PR is to add a Python coverage comment GitHub action to the CICD workflow, so that comments related to coverage change will be posted to a PR automatically.

In this PR:

  1. I updated the DEV_SETUP.md to include some instructions on how to enable pre-commit
  2. I added the coverage package as the dev dependency
  3. I updated the .coveragerc file so that .coverage is generated in relative path
  4. I added the workflow for generating and posting coverage comments.

Description

Contribution Checklist

@rewrlution rewrlution requested a review from a team as a code owner February 18, 2024 02:33
@shawncal shawncal added python Pull requests for the Python Semantic Kernel documentation labels Feb 18, 2024
@github-actions github-actions bot changed the title Pysdk issue 3980 Python: Pysdk issue 3980 Feb 18, 2024
@rewrlution
Copy link
Contributor Author

rewrlution commented Feb 19, 2024

Looks like "coverage comment action" is only supported in Linux machines:
image

@rewrlution
Copy link
Contributor Author

rewrlution commented Feb 19, 2024

For Linux machine, the execution result says there was something wrong with the permission settings:
image

Still investigating...

@rewrlution rewrlution force-pushed the pysdk-issue-3980 branch 3 times, most recently from 29cfd3a to 19d099a Compare February 20, 2024 17:47
@rewrlution
Copy link
Contributor Author

rewrlution commented Feb 20, 2024

Note that in this PR, I only enable the action on Linux machine and only for Python 3.11, since the code coverage remains the same for other os/version.

Also note that the ci failed because my github token does not have the necessary permission:
image

As you can see in the above image, even though I declare write permission for pull request in the ci workflow, my GITHUB_TOKEN still have the read only permission.

I use the same piece of code in a repo where I have the write permission, and it works. Therefore, I need someone who has the write permission to merge the code first.

Huijing Huang added 4 commits February 20, 2024 09:58
@rewrlution
Copy link
Contributor Author

rewrlution commented Feb 20, 2024

This is what I can see in my personal project, where I have the write permission:

image

@rewrlution
Copy link
Contributor Author

rewrlution commented Feb 20, 2024 •
edited
Loading

and here's the GITHUB TOKEN permission that I can see from my personal project:

Runner Image Provisioner
GITHUB_TOKEN Permissions
  Contents: write
  Metadata: read
  PullRequests: write

@moonbox3 moonbox3 mentioned this pull request Feb 20, 2024
Closed
4 tasks
@moonbox3 moonbox3 merged commit ce1ebe1 into microsoft:main Feb 22, 2024
moonbox3 added a commit to moonbox3/semantic-kernel that referenced this pull request Feb 22, 2024
@moonbox3
Revert "Python: Pysdk issue 3980 (microsoft#5063)"
dcb2b07 
This reverts commit ce1ebe1.
github-merge-queue bot pushed a commit that referenced this pull request Feb 22, 2024
@moonbox3
Python: Revert Pysdk issue 3980 (#5063) (#5119)
8079755 
This reverts commit ce1ebe1.

### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
LudoCorporateShark pushed a commit to LudoCorporateShark/semantic-kernel that referenced this pull request Aug 25, 2024
@rewrlution @moonbox3
Python: Pysdk issue 3980 (microsoft#5063)
f80b84e 
### Motivation and Context

This PR is to add a [Python coverage
comment](https://github.com/marketplace/actions/python-coverage-comment)
GitHub action to the CICD workflow, so that comments related to coverage
change will be posted to a PR automatically.

In this PR:
1. I updated the `DEV_SETUP.md` to include some instructions on how to
enable `pre-commit`
2. I added the `coverage` package as the dev dependency
3. I updated the `.coveragerc` file so that `.coverage` is generated in
relative path
4. I added the workflow for generating and posting coverage comments.

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  5. What problem does it solve?
  6. What scenario does it contribute to?
  7. If it fixes an open issue, please link to the issue here.
-->

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Huijing Huang <huijinghuang@microsoft.com>
Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com>
LudoCorporateShark pushed a commit to LudoCorporateShark/semantic-kernel that referenced this pull request Aug 25, 2024
@moonbox3
Python: Revert Pysdk issue 3980 (microsoft#5063) (microsoft#5119)
e8de071 
This reverts commit f80b84e.

### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
Bryan-Roe pushed a commit to Bryan-Roe-ai/semantic-kernel that referenced this pull request Oct 6, 2024
@rewrlution @moonbox3
Python: Pysdk issue 3980 (microsoft#5063)
d76aec6 
### Motivation and Context

This PR is to add a [Python coverage
comment](https://github.com/marketplace/actions/python-coverage-comment)
GitHub action to the CICD workflow, so that comments related to coverage
change will be posted to a PR automatically.

In this PR:
1. I updated the `DEV_SETUP.md` to include some instructions on how to
enable `pre-commit`
2. I added the `coverage` package as the dev dependency
3. I updated the `.coveragerc` file so that `.coverage` is generated in
relative path
4. I added the workflow for generating and posting coverage comments.

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  5. What problem does it solve?
  6. What scenario does it contribute to?
  7. If it fixes an open issue, please link to the issue here.
-->

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Huijing Huang <huijinghuang@microsoft.com>
Co-authored-by: Evan Mattson <35585003+moonbox3@users.noreply.github.com>
Bryan-Roe pushed a commit to Bryan-Roe-ai/semantic-kernel that referenced this pull request Oct 6, 2024
@moonbox3
Python: Revert Pysdk issue 3980 (microsoft#5063) (microsoft#5119)
fec39f8 
This reverts commit ce1ebe1.

### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eavanvalkenburg eavanvalkenburg eavanvalkenburg approved these changes

@moonbox3 moonbox3 moonbox3 approved these changes

Assignees

No one assigned

Labels

documentation python Pull requests for the Python Semantic Kernel

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rewrlution @eavanvalkenburg @moonbox3 @shawncal