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.

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

Mark issues as triaged when they are assigned #21790

Merged
merged 1 commit into from
Jun 15, 2022

Conversation

damccorm
Copy link
Contributor

@damccorm damccorm commented Jun 10, 2022

This automatically marks issues that have been assigned as not awaiting triage. Example of this working and removing the label when it should, and not removing it when it shouldn't

Part of #21684


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

See CI.md for more information about GitHub Actions CI.

@asf-ci
Copy link

asf-ci commented Jun 10, 2022

Can one of the admins verify this patch?

4 similar comments
@asf-ci
Copy link

asf-ci commented Jun 10, 2022

Can one of the admins verify this patch?

@asf-ci
Copy link

asf-ci commented Jun 10, 2022

Can one of the admins verify this patch?

@asf-ci
Copy link

asf-ci commented Jun 10, 2022

Can one of the admins verify this patch?

@asf-ci
Copy link

asf-ci commented Jun 10, 2022

Can one of the admins verify this patch?

@github-actions github-actions bot added the build label Jun 10, 2022
@@ -32,6 +32,7 @@ jobs:
echo "Assigning issue $ISSUE_NUMBER to $LOGIN"
echo "Using the link: https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER/assignees"
curl -H "Authorization: token $GITHUB_TOKEN" -d '{"assignees":["'"$LOGIN"'"]}' https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER/assignees
curl -X DELETE -H "Authorization: token $GITHUB_TOKEN" https://api.github.com/repos/$REPO/issues/$ISSUE_NUMBER/labels/awaiting%20triage
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: This might feel redundant since we're also assigning the issue, which might lead you to expect the other workflow to trigger and do this. That won't happen because GitHub doesn't trigger any events that were initiated by the github bot itself to infinite trigger loops.

@damccorm
Copy link
Contributor Author

R: @kennknowles

Copy link
Member

@kennknowles kennknowles left a comment

Choose a reason for hiding this comment

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

This seems fine. My first look at these files. Is shell script and curl really the best option for interacting here? Not a problem now but it raises the complexity bar for drop-in changes for a lot of people I think. Like python or javascript with a GH API library might be an option if things get any more complex?

@damccorm
Copy link
Contributor Author

damccorm commented Jun 10, 2022

Like python or javascript with a GH API library might be an option if things get any more complex?

Yeah, in general I agree (and that's what I've done for most actions things), for shorter ones like these bash is nice though because its one less layer of indirection. If we did anything beyond string matching and a curl request (which comes straight from the docs https://docs.github.com/en/rest/issues/labels#remove-a-label-from-an-issue), I'd want to move it out.

FWIW, Javascript has a built in gh client that is pretty easy to use (e.g.

await getGitHubClient().rest.issues.createComment({
), so that's the route I'd generally recommend for more complex stuff over python (which has one but its not released by GH)

@codecov
Copy link

codecov bot commented Jun 10, 2022

Codecov Report

Merging #21790 (913c81e) into master (d83da39) will not change coverage.
The diff coverage is 83.17%.

❗ Current head 913c81e differs from pull request most recent head b8b736e. Consider uploading reports for the commit b8b736e to get more accurate results

@@           Coverage Diff           @@
##           master   #21790   +/-   ##
=======================================
  Coverage   74.01%   74.01%           
=======================================
  Files         698      698           
  Lines       92224    92224           
=======================================
  Hits        68263    68263           
  Misses      22710    22710           
  Partials     1251     1251           
Flag Coverage Δ
go 50.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ks/python/apache_beam/runners/worker/opcounters.py 84.82% <66.66%> (ø)
sdks/python/apache_beam/transforms/cy_combiners.py 58.40% <69.56%> (ø)
...on/apache_beam/runners/direct/sdf_direct_runner.py 35.53% <75.00%> (ø)
sdks/python/apache_beam/runners/common.py 87.94% <75.90%> (ø)
sdks/python/apache_beam/utils/counters.py 86.74% <85.71%> (ø)
sdks/python/apache_beam/typehints/batch.py 86.23% <90.47%> (ø)
sdks/python/apache_beam/transforms/core.py 92.14% <92.00%> (ø)
...thon/apache_beam/ml/inference/sklearn_inference.py 92.50% <95.23%> (ø)
...eam/transforms/py_dataflow_distribution_counter.py 96.29% <100.00%> (ø)
sdks/python/apache_beam/transforms/window.py 87.05% <100.00%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d83da39...b8b736e. Read the comment docs.

@kennknowles kennknowles merged commit ceac85d into apache:master Jun 15, 2022
@damccorm damccorm deleted the users/damccorm/triage-on-assign branch June 15, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants