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.

Sign up for GitHub

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

feat: add release testing label to test failure reports #349

Merged
merged 3 commits into from
May 30, 2024
Merged

feat: add release testing label to test failure reports #349

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d28a5f
feat: add release testing label to test failure reports
mariajgrimaldi May 17, 2024
dc1e84a
refactor: add release testing label as default for test failure reports
mariajgrimaldi May 17, 2024
50f9573
refactor: address PR reviews
mariajgrimaldi May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions .github/workflows/add-label-on-failure-report.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@

name: Allows for the adding labels when opening a test failure issue.
name: Allows for adding labels when opening a test failure issue.

on:
issues:
Expand All @@ -8,9 +7,11 @@ on:
jobs:
add_label:
runs-on: ubuntu-latest
if: ${{ contains(github.event.issue.title, 'Test failure') && !contains(github.event.issue.labels.*.name, 'needs triage') }}
if: ${{ contains(github.event.issue.title, 'Test failure') && !contains(github.event.issue.labels.*.name, 'needs triage') && !contains(github.event.issue.labels.*.name, 'release testing') }}
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm reading the conditional correctly, this means that if the issue contains only one of 'needs triage' or 'release testing', it will not add the one that's missing. Is this intentional?

Copy link
Member Author

@mariajgrimaldi mariajgrimaldi May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. But I can rewrite so it adds the missing label. Also, I updated the test failure issue template so it includes both labels by default: dc1e84a. So now when opening an issue it'd be included as part of the default labels:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm all for simplication, so my instinct is to say we should remove it... but, on the other hand, whoever added it in the first place presumably ran into a situation that made it desirable. 🤷🏼‍♂️

If we're keeping it, though, I think it would be a good idea to add the missing label if only one is present.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we still need the job if we're using that as default? I'm guessing we do for people without enough permissions to add labels? I'm not familiar with the report view for people without any permissions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think we need it. Here's the discussion we had back then: #281 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done: 50f9573. Thanks for the feedback, @arbrandes!

steps:
- name: apply needs triage label
- name: Apply release testing labels
uses: actions-ecosystem/action-add-labels@v1
with:
labels: needs triage
labels: |
needs triage
release testing