-
Notifications
You must be signed in to change notification settings - Fork 39
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
Write github actions needed for auto closing issues opened beyond core #639
Conversation
c81634e
to
35fec8b
Compare
run: | | ||
gh issue edit $ISSUE_URL --add-project "Smackore" --add-label closed-by-membrane-bot | ||
sleep 10 | ||
export TICKET_ID=$(gh project item-list 19 --owner membraneframework --format json --limit 10000000 | elixir get_ticket_id.exs "$ISSUE_URL" | awk '/TICKET_ID/{print $2}') |
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.
export TICKET_ID=$(gh project item-list 19 --owner membraneframework --format json --limit 10000000 | elixir get_ticket_id.exs "$ISSUE_URL" | awk '/TICKET_ID/{print $2}') | |
export TICKET_ID=$(gh project item-list 19 --owner membraneframework --format json --limit 10000000 | elixir get_ticket_id.exs "$ISSUE_URL") |
why not just print ticket id without unnecessary stuff?
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.
Because .exs
script sometimes prints also logs from Mix.install/1
, like Resolving Hex dependencies...
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.
So that's another reason not to go with Elixir here ;)
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.
You could set MIX_QUIET=1
, though it seems it doesn't work for Mix.install
:P It may be worth to report an issue though
scripts/get_ticket_id.exs
Outdated
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.
Python is preinstalled in GH actions, why not just use it or jq
?
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.
Do you mean to use it instead of elixir?
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.
yes, setup-elixir is just an unnecessary cost here
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.
But does this cost hurt us anyhow?
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.
Frustration that action is working longer than it should. I don't think you'll saturate concurrency limits, given this runs on Linux runners only.
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'm leaving the issue of this script being written in Elixir to you. I'm not the owner of this code, so I'm not clicking accept, but this looks okay-ish to my eyes.
Relates to #622