[CI] Remove unnecessary PR validation

[ojhunt]

There's no reason to try to verify the PR number here - if it's not correct we error out anyway, so just ignore it entirely and pull the information in directly from github.

[llvmorg-github-actions]

@llvm/pr-subscribers-github-workflow

Author: Oliver Hunt (ojhunt)

Changes

There's no reason to try to verify the PR number here - if it's not correct we error out anyway, so just ignore it entirely and pull the information in directly from github.

---

Full diff: https://github.com/llvm/llvm-project/pull/199134.diff

1 Files Affected:

• (modified) .github/workflows/issue-write.yml (+47-51)

diff --git a/.github/workflows/issue-write.yml b/.github/workflows/issue-write.yml
index fd092477bd9e0..6d40ce85001b5 100644
--- a/.github/workflows/issue-write.yml
+++ b/.github/workflows/issue-write.yml
@@ -51,14 +51,10 @@ jobs:
           script: |
             var fs = require('fs');
             var comments = []
-            var pr_number = 0
             for (local_file of fs.readdirSync('.')) {
               if (local_file.startsWith("comments")) {
                 comments.push(...JSON.parse(fs.readFileSync(local_file)))
               }
-              if (local_file.startsWith("pr_number")) {
-                pr_number = parseInt(fs.readFileSync(local_file), 10)
-              }
             }
             if (!comments || comments.length == 0) {
               return;
@@ -72,67 +68,67 @@ jobs:
 
             console.log(runInfo);
 
+            var pr_number = 0
 
-            if (!pr_number) {
-              // Query to find the number of the pull request that triggered this job.
-              // The associated pull requests are based off of the branch name, so if
-              // you create a pull request for a branch, close it, and then create
-              // another pull request with the same branch, then this query will return
-              // two associated pull requests.  This is why we have to fetch all the
-              // associated pull requests and then iterate through them to find the
-              // one that is open.
-              const gql_query = `
-                query($repo_owner : String!, $repo_name : String!, $branch: String!) {
-                  repository(owner: $repo_owner, name: $repo_name) {
-                    ref (qualifiedName: $branch) {
-                      associatedPullRequests(first: 100) {
-                        nodes {
-                          baseRepository {
-                            owner {
-                              login
-                            }
+            // Query to find the number of the pull request that triggered this job.
+            // The associated pull requests are based off of the branch name, so if
+            // you create a pull request for a branch, close it, and then create
+            // another pull request with the same branch, then this query will return
+            // two associated pull requests.  This is why we have to fetch all the
+            // associated pull requests and then iterate through them to find the
+            // one that is open.
+            const gql_query = `
+              query($repo_owner : String!, $repo_name : String!, $branch: String!) {
+                repository(owner: $repo_owner, name: $repo_name) {
+                  ref (qualifiedName: $branch) {
+                    associatedPullRequests(first: 100) {
+                      nodes {
+                        baseRepository {
+                          owner {
+                            login
                           }
-                          number
-                          state
                         }
+                        number
+                        state
                       }
                     }
                   }
                 }
-              `
-              const gql_variables = {
-                repo_owner: runInfo.data.head_repository.owner.login,
-                repo_name: runInfo.data.head_repository.name,
-                branch: runInfo.data.head_branch
-              }
-              const gql_result = await github.graphql(gql_query, gql_variables);
-              console.log(gql_result);
-              // If the branch for the PR was deleted before this job has a chance
-              // to run, then the ref will be null.  This can happen if someone:
-              // 1. Rebase the PR, which triggers some workflow.
-              // 2. Immediately merges the PR and deletes the branch.
-              // 3. The workflow finishes and triggers this job.
-              if (!gql_result.repository.ref) {
-                console.log("Ref has been deleted");
-                return;
               }
-              console.log(gql_result.repository.ref.associatedPullRequests.nodes);
+            `
+            const gql_variables = {
+              repo_owner: runInfo.data.head_repository.owner.login,
+              repo_name: runInfo.data.head_repository.name,
+              branch: runInfo.data.head_branch
+            }
+            const gql_result = await github.graphql(gql_query, gql_variables);
+            console.log(gql_result);
+            // If the branch for the PR was deleted before this job has a chance
+            // to run, then the ref will be null.  This can happen if someone:
+            // 1. Rebase the PR, which triggers some workflow.
+            // 2. Immediately merges the PR and deletes the branch.
+            // 3. The workflow finishes and triggers this job.
+            if (!gql_result.repository.ref) {
+              console.log("Ref has been deleted");
+              return;
+            }
+            console.log(gql_result.repository.ref.associatedPullRequests.nodes);
 
-              gql_result.repository.ref.associatedPullRequests.nodes.forEach((pr) => {
+            gql_result.repository.ref.associatedPullRequests.nodes.forEach((pr) => {
+
+              // The largest PR number is the one we care about.  The only way
+              // to have more than one associated pull requests is if all the
+              // old pull requests are in the closed state.
+              if (pr.baseRepository.owner.login === context.repo.owner && pr.number > pr_number) {
+                pr_number = pr.number;
+              }
+            });
 
-                // The largest PR number is the one we care about.  The only way
-                // to have more than one associated pull requests is if all the
-                // old pull requests are in the closed state.
-                if (pr.baseRepository.owner.login = context.repo.owner && pr.number > pr_number) {
-                  pr_number = pr.number;
-                }
-              });
-            }
             if (pr_number == 0) {
               console.log("Error retrieving pull request number");
               return;
             }
-            
+
             await comments.forEach(function (comment) {
               if (comment.id) {
                 // Security check: Ensure that this comment was created by

[ojhunt]

I guess we could retain the original pr logic to report an error on mismatch? not sure if it's worth it though?

[lukel97]

The pr_number file isn't for validation, it's needed for #190010. The issue_write event doesn't expose the PR number in the payload so it needs to be plumbed through manually

See #192205 for the rationale

[ojhunt]

The pr_number file isn't for validation, it's needed for #190010. The issue_write event doesn't expose the PR number in the payload so it needs to be plumbed through manually

See #192205 for the rationale

Well that would certainly do it - I saw the error(?) case and assumed it must always be available. Seems like closing this is the right thing to do?

[boomanaiden154]

Yeah, closing this is probably the most reasonable course of action.

[ojhunt]

Closing, reason: I am a muppet :D