Skip to content

Make searches paginated #474

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 8 commits into from
Apr 29, 2022

Conversation

414owen
Copy link
Contributor

@414owen 414owen commented Jan 17, 2022

Previously, searches only returned the first thirty results, and there was no way to access page two.

Note that this is a breaking API change.

@414owen 414owen force-pushed the paginated-searches branch from 11332ed to 4d8d44a Compare January 17, 2022 15:35
@andreasabel
Copy link
Member

Excellent!
There should be something added to the testsuite using the new API. Can you look into adding tests, @414owen?

@andreasabel
Copy link
Member

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Apr 19, 2022

rebase

✅ Branch has been successfully rebased

Copy link
Member

@andreasabel andreasabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please supply a testcase or a demo in samples/.

@414owen
Copy link
Contributor Author

414owen commented Apr 19, 2022

@andreasabel I've fixed up the existing samples, will this do?

Copy link
Member

@andreasabel andreasabel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!
Could you please add the samples to the executables in samples/github-samples.cabal so that they get built by the CI.
Concerning the tests you added, unfortunately they do not run at the moment on CI, see https://github.com/haskell-github/github/runs/6098329674?check_suite_focus=true#step:20:144 and issue

import Control.Monad (forM,forM_)
import Data.Maybe (fromMaybe)
import qualified Github as Github
import qualified Github as Github
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you doubled this line by accident.

@414owen
Copy link
Contributor Author

414owen commented Apr 21, 2022

Trying to use the github-samples.cabal project locally errors with Could not find module ‘Github’, but I presume CI will have more luck.

change needed: s/Github/GitHub/g

@414owen 414owen force-pushed the paginated-searches branch from 98d4b9a to 73b301e Compare April 29, 2022 12:00
@414owen
Copy link
Contributor Author

414owen commented Apr 29, 2022

@andreasabel Looks like code search is broken in master, because the repository field doesn't line up 1:1 with the Repo datatype.

I've fixed it in this branch, by adding a new CodeSearchRepo type.

@414owen 414owen force-pushed the paginated-searches branch from 73b301e to 3512972 Compare April 29, 2022 12:13
@andreasabel andreasabel added this to the 0.28 milestone Apr 29, 2022
@andreasabel
Copy link
Member

Weird error message by GHC 8 and 9 in CI failure (https://github.com/haskell-github/github/runs/6228048459?check_suite_focus=true#step:18:153):

Building executable 'github-search-repos' for github-samples-0..
[1 of 1] Compiling Main             ( Search/SearchRepos.hs, /__w/github/github/dist-newstyle/build/x86_64-linux/ghc-9.2.2/github-samples-0/x/github-search-repos/noopt/build/github-search-repos/github-search-repos-tmp/Main.o )

Search/SearchRepos.hs:43:34: error:
    Ambiguous occurrence ‘GitHub.repoName’
    It could refer to
       either the field ‘repoName’,
              imported qualified from ‘GitHub’ at Search/SearchRepos.hs:4:1-23
              (and originally defined in ‘GitHub.Data.Repos’)
           or the field ‘repoName’,
              imported qualified from ‘GitHub’ at Search/SearchRepos.hs:4:1-23
              (and originally defined in ‘GitHub.Data.Repos’)
   |
43 |   let fields = [ ("Name", show . GitHub.repoName)
   |                                  ^^^^^^^^^^^^^^^

GHC 7.8 / 7.10 fail with (https://github.com/haskell-github/github/runs/6228049019?check_suite_focus=true#step:18:46):

Building library for github-0.27.1..

src/GitHub/Data/Repos.hs:3:14:
    Unsupported extension: DuplicateRecordFields
@@ -1,5 +1,6 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE DuplicateRecordFields #-}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe don't use the DuplicateRecordFields extension. Are there strong reasons to use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, I used DuplicateRecordFields so I could copy-paste Repo fields into CodeSearchRepo fields. I'll remove it now.

@414owen
Copy link
Contributor Author

414owen commented Apr 29, 2022

Ugh, GHC-7 semigroup issue -_-
One sec...

@andreasabel
Copy link
Member

Thanks, @414owen , looks like everything is ready. Squashing is ok? (Since there are fixup commits...)

@andreasabel andreasabel self-assigned this Apr 29, 2022
@414owen
Copy link
Contributor Author

414owen commented Apr 29, 2022

Sure thing!

@andreasabel andreasabel merged commit 83acdf8 into haskell-github:master Apr 29, 2022
andreasabel added a commit that referenced this pull request Apr 30, 2022
andreasabel added a commit that referenced this pull request Apr 30, 2022
andreasabel added a commit that referenced this pull request Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants