Skip to content

Find a forked repository in GitHub search #72

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

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

chaspy
Copy link
Contributor

@chaspy chaspy commented Apr 4, 2024

Thank you for a great tool!

Problem to be solved

rails/web-console is currently not detectable by dep-doctor.

[warn] web-console (unknown):

This is because the GraphQL Query cannot discover what has been forked.
Here are the results in Explorer.

request:

query {
  search(query: "repo:rails/web-console", type: REPOSITORY, first: 10) {
    edges {
      node {
        ... on Repository {
          isArchived
          nameWithOwner
          isMirror
          url
          owner {
            login
          }
          defaultBranchRef{
            target{
              ... on Commit {
               history(first: 1) {
                edges{
                  node{
                    committedDate
                  }
                }
              } 
              }
            }
          }
        }
      }
    }
  }
}

response

{
  "data": {
    "search": {
      "edges": []
    }
  }
}

with fork:true , we can get the result as expected.

query {
  search(query: "repo:rails/web-console fork:true", type: REPOSITORY, first: 10) {
    edges {
      node {
        ... on Repository {
          isArchived
          nameWithOwner
          isMirror
          url
          owner {
            login
          }
          defaultBranchRef{
            target{
              ... on Commit {
               history(first: 1) {
                edges{
                  node{
                    committedDate
                  }
                }
              } 
              }
            }
          }
        }
      }
    }
  }
}

result

{
  "data": {
    "search": {
      "edges": [
        {
          "node": {
            "isArchived": false,
            "nameWithOwner": "rails/web-console",
            "isMirror": false,
            "url": "https://github.com/rails/web-console",
            "owner": {
              "login": "rails"
            },
            "defaultBranchRef": {
              "target": {
                "history": {
                  "edges": [
                    {
                      "node": {
                        "committedDate": "2023-12-08T18:04:17Z"
                      }
                    }
                  ]
                }
              }
            }
          }
        }
      ]
    }
  }
}

I built dep-doctor with this change, rails/web-console is diagnosed expectedly.

Could you review? and let me run the CI 🙏

Thanks!

@chaspy chaspy marked this pull request as draft April 4, 2024 01:24
@chaspy chaspy changed the title Add test Find a forked repository in GitHub search Apr 4, 2024
@chaspy chaspy marked this pull request as ready for review April 4, 2024 01:31
@chaspy
Copy link
Contributor Author

chaspy commented Apr 4, 2024

@kyoshidajp
FetchFromGitHub function looks like mocked. (GitHub api is not actually executed)
Should I add a test somewhere?

@chaspy chaspy marked this pull request as draft April 4, 2024 04:18
like rails/web-console is forked.
So we cannot get the result with a current query.
@chaspy chaspy marked this pull request as ready for review April 4, 2024 04:45
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8549370946

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 75.729%

Totals Coverage Status
Change from base Build 7046933064: 0.0%
Covered Lines: 649
Relevant Lines: 857

💛 - Coveralls

@kyoshidajp kyoshidajp merged commit 05ebea5 into kyoshidajp:main Apr 5, 2024
7 checks passed
@kyoshidajp
Copy link
Owner

@chaspy Thank you for your contributing! The implementation of FetchFromGitHub is not mandatory. Since the CI also succeeded, merged it.

I will release new version in this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants