[GitHub] [beam] damccorm commented on a diff in pull request #22032: Enable close issue as not planned

[GitBox <gi...@apache.org>]

damccorm commented on code in PR #22032:
URL: https://github.com/apache/beam/pull/22032#discussion_r905436484


##########
.github/workflows/self-assign.yml:
##########
@@ -34,10 +35,26 @@ jobs:
           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
         fi
-        if [[ $BODY == *"$INPUT_CLOSE"* && $BODY != *"\`$INPUT_CLOSE\`"* ]]; then
-          echo "Closing issue $ISSUE_NUMBER"
+        INPUT_CLOSE_BRACKET="$INPUT_CLOSE"'\(([^()]*)\)'
+        INPUT_CLOSE_BRACKET_BACKTICK="\`$INPUT_CLOSE_BRACKET\`"
+        ISSUE_CLOSE_CONFIG=""
+        if [[ ! $BODY =~ $INPUT_CLOSE_BRACKET_BACKTICK ]]; then
+          if [[ $BODY =~ $INPUT_CLOSE_BRACKET ]]; then
+            if [[ ${BASH_REMATCH[1]} == "completed" ]] || [[ ${BASH_REMATCH[1]} == "not_planned" ]]; then
+              # close issue with specified reason. Supported reasons are "completed" or "not_planned".
+              ISSUE_CLOSE_CONFIG='"state":"closed","state_reason":"'${BASH_REMATCH[1]}\"
+            else
+              echo "Unknown issue close reason: ${BASH_REMATCH[1]}"
+            fi
+          elif [[ $BODY == *"$INPUT_CLOSE"* && $BODY != *"\`$INPUT_CLOSE\`"* ]]; then
+            # Close issue without specify a reason. Will be closed as complete by default.
+            ISSUE_CLOSE_CONFIG='"state":"closed"'
+          fi
+        fi
+        if [[ -n $ISSUE_CLOSE_CONFIG ]]; then

Review Comment:
   ```suggestion
           if [[ $BODY == *"$INPUT_CLOSE"* && $BODY != *"\`$INPUT_CLOSE\`"*  ]]; then
              $ISSUE_CLOSE_CONFIG='"state":"closed"'
              if [[ $BODY == *"not_planned"* ]]; then
                 $ISSUE_CLOSE_CONFIG='"state":"closed","state_reason":"not_planned"'
              fi
   ```
   
   I think we can vastly simplify this. This uses 3 things:
   1) anything closed is already closed as completed. So we don't have to check that.
   2) anything closed as not planned will have the not_planned suffix somewhere and its really unlikely to appear elsewhere (and the cost of messing that up is low).
   3) If they enter an invalid input (e.g. `.close-issue(abc)`) closing is better than silently failing (which is currently what you're trying to do.
   
   IMO simplicity is much better than trying to make this 100% perfect. This is already more complex than I'd like as a bash script and probably should be migrated to a javascript action soon



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org