sindresorhus/refined-github

`show-open-prs-of-forks` includes unrelated PRs #2934

fregante posted onGitHub

  1. Visit one of your (source) repos where you opened PRs
  2. Fork it
  3. Open it
  4. Notice: your existing PRs are counted in this feature

In my case, I have this dummy fork https://github.com/fregante/refined-github but all of PRs are directly on the source (https://github.com/sindresorhus/refined-github)

<img width="364" alt="" src="https://user-images.githubusercontent.com/1402241/77570114-91196b00-6ecb-11ea-8762-a48694535e69.png">

cc @loilo


Fair point. I'll take care of it. 🙂

posted by loilo about 5 years ago

Oof. From what I can tell, GitHub has no search filter in place to limit results to a certain head repo.

This creates two problems:

  • We can't determine the exact number of open PRs without fetching a lot more data from the API than we do now. (At least I think we can't. There may be a possibility to utilize the GraphQL API in a way which can filter for certain nested node values. But since I have no considerable knowledge of GraphQL, I can't tell whether or not this is possible.)
  • We also can't reliably link to a list of open PRs since that link is entirely composed of issue search filters.

I'm not sure how to handle this.

We could solve part of it by checking if the user has write access not only to the forked repo but also to the source repo. This should be solvable in one API request and could be cached for a long time.

  • If the user has no write access to the source repo, we should be fine keeping the current process in place.
  • If the user has write access to the source repo (as in your reproduction setup), we may consider fetching individual pull requests from the API and filtering them by head repo on the client side. We then could forego linking to a list of PRs and just tell the user about the fact that there are open PRs.

What do you think @fregante?

posted by loilo about 5 years ago

Here: https://developer.github.com/v4/explorer/ We'd have to fetch all your PR info, not just the first one, but it's not a lot of data.

query { 
        search(
            first: 100,
            type: ISSUE,
            query: "repo:sindresorhus/refined-github is:pr is:open author:fregante"
        ) {
            nodes {
                ... on PullRequest {
                    baseRepository {
                        nameWithOwner
                    }
                    number
                }
            }
        }
}
{
  "data": {
    "search": {
      "nodes": [
        {
          "baseRepository": {
            "nameWithOwner": "sindresorhus/refined-github"
          },
          "number": 2925
        }
      ]
    }
  }
}

Then something like:

const prs = search.nodes.filter(pr => pr.baseRepository.nameWithOwner === getRepoURL());
posted by fregante about 5 years ago

Yes, I had tried it out in the explorer, that's basically how I imagined it to be implemented. The "lot" of data was more in comparison with before, and it grows linearly with number of PRs.

I think this would be a reasonable implementation. In the (probably a bit absurd) case that somebody has > 100 open PRs, I'd bail out of counting.

How'd you deal with the link problem?

posted by loilo about 5 years ago

I don’t expect anyone to have a considerable amount of PRs open from a fork, they’re distant outliers.

As for the link, there’s no direct fix, but:

  • I’d expect people to open PRs from the fork OR the source, not at the same time (generally)
  • if they do have mixed repos, our pr-branches feature will help them understand which is which

Alternatively, I think that the search accepts searching by number and logical operators. If that works, we could link to the search issue:2 OR issue:30 OR issue:40 Not possible

posted by fregante about 5 years ago

Doesn't seem to be possible though. 🙁

posted by loilo about 5 years ago

If some one wants to send a PR I tested https://github.com/sindresorhus/refined-github/issues/2934#issuecomment-604146813 and it looks fine to me. (I changed it to headRepository because I think that is correct) https://github.com/yakov116/refined-github/commit/b698cf3f3e8f2f5d38e815f4eedbd2cb9aa47888

@@ -17,24 +17,28 @@ const countPRs = cache.function(async (forkedRepo: string): Promise<[number, num
     // This allows to link to the PR directly if only one is found
     const {search} = await api.v4(`
         search(
-            first: 1,
+            first: 100,
             type: ISSUE,
             query: "repo:${forkedRepo} is:pr is:open author:${getUsername()}"
         ) {
-            issueCount
             nodes {
                 ... on PullRequest {
                     number
+                    headRepository {
+                        nameWithOwner
+                    }
                 }
             }
         }
     `);

-    if (search.issueCount === 1) {
-        return [1, search.nodes[0].number];
+    const prs = search.nodes.filter((pr: AnyObject) => pr.headRepository.nameWithOwner.toLowerCase() === getRepoURL());
+
+    if (prs.length === 1) {
+        return [1, prs[0].number];
     }

-    return [search.issueCount];
+    return [prs.length];
 }, {
     maxAge: 1 / 2, // Stale after 12 hours
     staleWhileRevalidate: 2,
posted by yakov116 almost 5 years ago

Fund this Issue

$0.00
Funded

Pull requests