27: Add pagination support (Elixir)#30
27: Add pagination support (Elixir)#30murjax wants to merge 1 commit intoelixir-repos-in-folder-22from
Conversation
| |> List.first | ||
| |> GithubRepoCloner.Cloner.clone | ||
| username = List.first(args) | ||
| PageIterator.repeat(&Cloner.clone_page/1, %{username: username, page: 1}) |
There was a problem hiding this comment.
Had to rethink the cloner logic here. I could have simply tossed the existing logic in a loop that runs until we reach an empty page. That increases the cyclomatic complexity, or in other words the number of scenarios the function handles. I decided it was better to isolate the logic for cloning one page of repos (Cloner) and extract the logic for handling pagination elsewhere (PageIterator).
|
|
||
| defp run_command(command) do | ||
| @system.cmd("sh", ["-c", command]) | ||
| {:ok, @system.cmd("sh", ["-c", command])} |
There was a problem hiding this comment.
Return an ok tuple if the clone command is run.
| end | ||
|
|
||
| defp run_command("") do | ||
| {:error, "No repositories found"} |
There was a problem hiding this comment.
Return an error tuple if the command is not run. This includes scenarios where no repos are found for the username or page and if the username is invalid.
| @@ -0,0 +1,12 @@ | |||
| defmodule GithubRepoCloner.PageIterator do | |||
There was a problem hiding this comment.
Rules of the page iterator:
- Accept an anonymous function and an arguments map that will be passed to the function.
- The arguments map must include a
page. - Recursively call the function while it returns an
oktuple, increasing the page number on each call. - Stop recursion whenever an
errortuple is returned. - Page iterator returns the last successful page reached in an
oktuple at the end.
There was a problem hiding this comment.
write these in as a comment in this code.
There was a problem hiding this comment.
if you felt the need to add it here just add it as a comment in the code.
There was a problem hiding this comment.
At the very least leave a comment describing the base condition that terminates the repeat iterator.
murjax
changed the title
| |> List.first | ||
| |> GithubRepoCloner.Cloner.clone | ||
| username = List.first(args) | ||
| PageIterator.repeat(&Cloner.clone_page/1, %{username: username, page: 1}) |
There was a problem hiding this comment.
I think the name should change to clone_pages ?
There was a problem hiding this comment.
Might wrap this in a new Cloner.clone_pages function. CLI will use Cloner.clone_pages.
MichaelDimmitt
left a comment
MichaelDimmitt
left a comment
There was a problem hiding this comment.
left some comments
2 participants
Issue #27 for Elixir.