-
Notifications
You must be signed in to change notification settings - Fork 12
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
chore: issue labeler workflow #118
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
==========================================
- Coverage 84.60% 84.53% -0.08%
==========================================
Files 198 198
Lines 35400 35400
Branches 4569 4546 -23
==========================================
- Hits 29951 29926 -25
- Misses 5295 5326 +31
+ Partials 154 148 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: github-actions <github-actions@github.com>
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.
There are a lot of difference between the resulting workflow and the one in the aws-cdk repo. If those are expected, please add a note. If not, please fix :)
parameters?: string; | ||
affixes?: string; | ||
areaIsKeyword?: boolean; | ||
needEnvs?: boolean; |
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.
How come one jobs needs it and one doesn't? I see its configured on the workflow level in the aws-cdk repo.
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.
its configured on the workflow level, yes. but it doesn't look like the other job uses those env variables, even if it might have access to it. because of that i think its fine to have the env variables be configured to the job level
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.
i didn't add the guidance step, which would utilize the workflow level env vars (probs why they put it on the workflow level).
however, projen doesn't currently support workflow-level env variables, so if we were to include the guidance job i'd just add needEnvs: true
to that step for now (until we have projen support).
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.
Followup then - do we not need the guidance step?
|
||
this.workflow.addJob('Triage Issues', triageManagerJob({ | ||
target: 'issues', | ||
excludedExpressions: ['CDK CLI Version', 'TypeScript', 'Java', 'Python', 'Go'], |
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.
Are you sure this works? In the aws-cdk repo this is configured as a string.
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.
missed a cnversion step :(
target: 'issues', | ||
excludedExpressions: ['CDK CLI Version', 'TypeScript', 'Java', 'Python', 'Go'], | ||
includedLabels: ['needs-triage'], | ||
excludedLabels: ['p1', 'p2', 'p0', 'effort-small', 'effort-medium', 'effort-large', 'guidance'], |
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.
same here
uses: aws-github-ops/aws-issue-triage-manager@main | ||
with: | ||
target: pull-requests | ||
areaIsKeyword: true |
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.
Casing doesn't bother the action? See https://github.com/aws/aws-cdk/blob/main/.github/workflows/issue-label-assign.yml#L36C11-L36C26
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.
it definitely does, i just missed doing a conversion step that i had planned
Grrr, no projen support: projen/projen#3567. I'm lazy so I just copied and pasted a few from the aws-cdk repo. This is somewhat necessary because it adds the `needs-triage` label automatically, that my other PR will need: #118 --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
- name: Triage Manager | ||
uses: aws-github-ops/aws-issue-triage-manager@main | ||
with: | ||
github-token: ${{ secrets.PROJEN_GITHUB_TOKEN }} |
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.
https://github.com/aws/aws-cdk/pull/19727/files this doesn't tell me why but it seems it was intentional
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.
i believe the issue-label-assign workflow has the equivalent job to what exists on aws-cdk now
Signed-off-by: github-actions <github-actions@github.com>
Attempting to copy and projenify this workflow: https://github.com/aws/aws-cdk/blob/main/.github/workflows/issue-label-assign.yml
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license