-
-
Notifications
You must be signed in to change notification settings - Fork 774
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
GHA add comments to prework issue 4820 #6873
GHA add comments to prework issue 4820 #6873
Conversation
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes.
|
Hey @marioantonini, when you have a moment please add your ETA and availability. Thanks! |
Hey @marioantonini @jphamtv any update on this PR? I'm hoping to get some feedback and close it so I can work on more issues. Thank you! |
@partapparam @jphamtv Totally missed this, will get to it today. |
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.
Hey @partapparam Thanks for working on this issue! Great job so far- I started testing in my own repo. These are my comments so far:
For a test Pull Request:
-
In
pull-request-trigger.yml
, line 70, the section of the if statementgithub.event_name == 'pull_request'
is able to be removed since the yml is triggered by the pull request, e.g. line 70 can beif: ${{ github.event.action == 'opened' || github.event.action == 'closed' }}
-
In
prework-issue-comment-reusable-workflow.yaml
, the workflow failed right away because of the "if" statements in the "script" - I think you can get around this by placing your "if" statements at the head of each step, in front of "name" - I made some edits and got through this snag -
When
prework-issue-comment-reusable-workflow.yaml
callsget-activity-detail.js
, the workflow get snagged because on line 23, the event is "pull_request_target" rather than on "pull_request". -
I tweaked this and the following run was snagged on the next step "Get prework issue" - though I am not sure why yet.
-
Also, in the "Move issue" step, the workflow got snagged by "const" as shown:
-
I know that the original issue requested all of the conditions you are checking for. When this is running we can check for certain however I think that some of the conditions are overlapping each other. We can look at this later....
-
FYI, CodeQL has flagged an "Unused variable" in
get-activity-details.js
see above and in "Files changed"
Thanks again
From our previous conversation on Slack: Sorry for the short notice- this is happening very quickly. Each of you have currently submitted a PR that involves GitHub Actions and wanted to let you know that we will need to hold off merging any new GHAs until after the migration. To make this interesting, also need to let you know that after migration, any of the existing API calls using a ‘column’ reference will no longer work because Projects Beta is not structured the same- the new term is ‘status’ and it is not interchangeable. Some other REST API endpoints that will be deprecated soon are here. So for all of you, after migration your workflows will need to be tested using Projects Beta. If you already installed Projects (classic) on your repo you can migrate to Projects Beta for early testing. (FYI after migrating you can reopen Projects (classic)- but do that immediately if you want to keep it because that option disappeared for me after a day.) For any workflows that currently involve moving an issue to or from a ‘column’, this should be doable using GraphQL, ProjectV2, and Status. (I have not done this yet- still learning!) And/or we can discuss other options. Finally, one last thing I notice is that for all of you there are parts of your issue that overlap with the other two’s issues. As one example, all three of your issues include posting messages to a member’s “Pre-work Checklist”. Unfortunately, this should have been caught earlier so that each issue was worked on after the other was completed, so that you would not each have a different solution to the same problem. (I could also be wrong and maybe there will not be any conflicts.) |
From 6/24 Slack |
Hey @partapparam I know you have put a lot of work into this, and after we went through the projects migration much has changed. Were you planning to work on this again? Is there anything we should talk about/ discuss? Please let me know - thanks! |
Hey @t-will-gillis, this slipped my radar. I'll review and push the updates this week. Thanks! |
@partapparam - Please add your availability and ETA to make the requested changes. Thanks! |
Availability and ETA: Sunday, 9/15, 3-9 pm. ETA 9/22. Having some issues with this. I'll try and commit some updates to this on Sunday to get feedback. |
Hey @partapparam, could you provide an updated ETA and your availability for this PR? If you need assistance, please add a comment with the 'Status: Help Wanted' label or reach out to the team on Slack. Thanks! |
Param has unassigned themselves from the issue related to this PR. See #4820 (comment) |
@partapparam Thank you for the work that you put into this! |
Fixes #4820
What changes did you make?
The following event/activity types are set up to trigger the workflow
opened
,assigned
,unassigned
,closed as completed
, orclosed as not planned
issue-trigger.yml
file with new jobAdd-Comment-To-Prework-Issue
created
issue-comment-trigger.yaml
file and created new jobAdd-Comment-To-Prework-Issue
opened
,closed
pull-request-trigger.yaml
file with new jobAdd-Comment-To-Prework-Issue
submitted
pull-request-review-trigger.yaml
file and created new jobAdd-Comment-To-Prework-Issue
created
pull-request-review-comment-trigger.yaml
file and created new jobAdd-Comment-To-Prework-Issue
Created
prework-issue-reusable-workflow
folder ingithub-actions
Added files to
github-actions/utils
folderCreated
prework-issue-comment-reusable-workflow.yaml
with the following steps:get-activity-detail.js
get-prework-issue.js
andget-issue-by-label.js
update-prework-issue-status.js
andreopen-issue.js
update-issue-project-card.js
add-prework-issue-comment.js
Why did you make the changes (we will use this info to test)?
Notes
prework-issue-comment-reusable-workflow.yaml
are located in thegithub-actions/prework-issue-reusable-workflow
folderpermissions: issues: write
but I believe this is not required in this repo. I can remove once tested by the reviewer.