-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix/update comment #184
Fix/update comment #184
Conversation
Codecov ReportPatch and project coverage have no change.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #184 +/- ##
=======================================
Coverage 98.42% 98.42%
=======================================
Files 15 15
Lines 444 444
=======================================
Hits 437 437
Misses 7 7 ☔ View full report in Codecov by Sentry. |
Good suggestion! If we don't have a strong need for keeping results from non-PR commits in AWS, then I'll go for saving storage and processing time. |
Looks great, but I'm confused why this PR doesn't have the analyst and comment on it |
.github/workflows/analyze.yml
Outdated
- name: Check if PR | ||
id: check | ||
run: | | ||
if [[ "${{ github.event.pull_request }}" != "" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like github.event.pull_request
is empty for this PR, which results in this statement evaluating to False
. Maybe test it by checking what is the value stored in github.event.pull_request
?
Packing analysis reportAnalysis for packing results located at cellpack/tests/outputs/test_spheres/spheresSST
Packing imageDistance analysisExpected minimum distance: 50.00
|
Ran the following tests:
This is expected behavior and we should be good to merge! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, wait for #185 to be merged.
…/update-comment
Problem
What is the problem this work solves, including
#176
Solution
What I/we did to solve this problem
the workflow will on be triggered when:
main
Type of change
Please delete options that are not relevant.
Steps to Verify: