Skip to content
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

Add support for listing PRs in need of review #134

Merged
merged 8 commits into from
Jul 5, 2024
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ node_modules/
harmony-npm/

config.json
harmony.json
harmony
harmony-npm.tar.gz

Expand Down
8 changes: 5 additions & 3 deletions src/BashCompletion.idr
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,16 @@ cmdOpts "pr" partialArg "pr" =
else if isHashPrefix partialArg
then Nothing -- <- allows us to fall through to handle with config below.
else Just []
cmdOpts "contribute" "-" _ = Just ["--checkout", "-c", "--ignore", "-i"]
cmdOpts "contribute" "--" _ = Just ["--checkout", "-c", "--ignore", "-i"]
cmdOpts "contribute" "-" _ = Just ["--checkout", "-c", "--list", "-l", "--ignore", "-i"]
cmdOpts "contribute" "--" _ = Just ["--checkout", "-c", "--list", "-l", "--ignore", "-i"]
cmdOpts "contribute" partialArg _ =
if partialArg `isPrefixOf` "--checkout"
then Just ["--checkout"]
else if partialArg `isPrefixOf` "--ignore"
then Just ["--ignore"]
else Just []
else if partialArg `isPrefixOf` "--list"
then Just ["--list"]
else Just []
cmdOpts "graph" "--" _ = Nothing
cmdOpts "graph" "-" _ = Just ["--completed", "-c"]
cmdOpts "graph" partialArg _ =
Expand Down
61 changes: 51 additions & 10 deletions src/Commands.idr
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ parseGraphArgs args =

data IgnoreOpt = PRNum Nat

data ContributeArg = Checkout | Ignore IgnoreOpt | Skip Nat
data ContributeArg = List | Checkout | Ignore IgnoreOpt | Skip Nat

skipArg : ContributeArg -> Maybe Nat
skipArg (Skip n) = Just n
Expand All @@ -252,7 +252,7 @@ ignorePRNums (_ :: xs) = ignorePRNums xs

contributeUsageError : String
contributeUsageError =
"contribute's arguments must be -<num> to skip num PRs, --ignore (-i) <uri>/<pr-number>, or --checkout (-c) to checkout the branch needing review."
"contribute's arguments must be -<num> to skip num PRs, --ignore (-i) <uri>/<pr-number>, --list to list PRs instead of picking the first one, or --checkout (-c) to checkout the branch needing review."

||| Parse arguments to the contribute subcommand.
export
Expand All @@ -261,9 +261,14 @@ parseContributeArgs [] = Right []
parseContributeArgs args =
let (ignoreArgs, rest) = recombineIgnoreArgs args
ignoreArgs' = Ignore <$> ignoreArgs
rest' = (traverse (parseSkipArg <||> parseCheckoutFlag) rest)
rest' = (traverse (parseListFlag <||> parseSkipArg <||> parseCheckoutFlag) rest)
in maybeToEither contributeUsageError ((ignoreArgs' ++) <$> rest')
where
parseListFlag : String -> Maybe ContributeArg
parseListFlag "-l" = Just List
parseListFlag "--list" = Just List
parseListFlag _ = Nothing

parseCheckoutFlag : String -> Maybe ContributeArg
parseCheckoutFlag "-c" = Just Checkout
parseCheckoutFlag "--checkout" = Just Checkout
Expand Down Expand Up @@ -315,26 +320,62 @@ contribute @{config} args = do
if (null newIgnorePRNums)
then pure config
else addIgnoredPRs config newIgnorePRNums
-- options:
let skip = fromMaybe 0 (head' $ mapMaybe skipArg args)
let checkout = find (\case Checkout => True; _ => False) args
let list = find (\case List => True; _ => False) args
when (isJust checkout && isJust list) $
reject "The --checkout and --list options are mutually exclusive"
-- execution:
let notMine = filter (not . isAuthor myLogin) nonDraftPrs
let notIgnored = filter (isNotIgnored config') notMine
let parted = partition (isRequestedReviewer myLogin) notIgnored
let (requestedOfMe, others) = (mapHom $ sortBy (compare `on` .createdAt)) parted
let pr = head' . drop skip $ requestedOfMe ++ others
case pr of
Nothing => reject "No open PRs to review!"
Just pr => do
whenJust (checkout $> pr.headRef) $ \branch => do
checkoutBranch branch
putStrLn (pr.webURI @{config'})
case list of
Nothing => pickOne config' skip checkout requestedOfMe others
(Just _) => listSome skip requestedOfMe others

where
isNotIgnored : Config -> PullRequest -> Bool
isNotIgnored config pr =
isNothing $
find (== pr.number) config.ignoredPRs

pickOne : Config -> Nat -> Maybe ContributeArg -> List PullRequest -> List PullRequest -> Promise' ()
pickOne config' skip checkout requestedOfMe others =
let pr = head' . drop skip $ requestedOfMe ++ others
in case pr of
Nothing => reject "No open PRs to review!"
Just pr => do
whenJust (checkout $> pr.headRef) $ \branch => do
checkoutBranch branch
putStrLn (pr.webURI @{config'})

printDetails : PullRequest -> Doc AnsiStyle
printDetails pr =
let branch = annotate italic (pretty pr.headRef)
title = pretty " ├ title: " <++> pretty pr.title
created = pretty " ├ created:" <++> pretty (show pr.createdAt)
number = pretty " ├ number: " <++> annotate (color Green) (pretty pr.number)
link = pretty " └ url: " <++> annotate (color Blue) (pretty pr.webURI)
in vsep [branch, title, created, number, link]

goListSome : Bool -> List PullRequest -> Promise' ()
goListSome direct prs = do
let (pr :: rest) = prs
| [] => pure ()
let first = printDetails pr
renderIO $
indent (if direct then 0 else 2) $
foldl (\acc, next => vsep [acc, emptyDoc, printDetails next]) first rest

listSome : Nat -> List PullRequest -> List PullRequest -> Promise' ()
listSome skip requestedOfMe others = do
goListSome True (drop skip requestedOfMe)
putStrLn ""
renderIO $ annotate bold "Your review not requested:"
goListSome False (take 5 others)

||| Print the GitHub URI for the current branch when the user
||| executes `harmony branch`.
export
Expand Down
38 changes: 21 additions & 17 deletions src/Data/PullRequest.idr
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,27 @@ public export
record PullRequest where
constructor MkPullRequest
||| The pull request's "number" (as seen in URIs referring to the PR).
number : Integer
number : Integer
||| The pull request's title
title : String
||| When the PR was created.
createdAt : Date
createdAt : Date
||| Is the PR currently a "draft"?
isDraft : Bool
isDraft : Bool
||| The `login` of the author of the pull request.
author : String
author : String
||| Open or Closed status.
state : PRState
state : PRState
||| A List of all reviewers requested on the PR.
reviewers : List String
reviewers : List String
||| The branch being merged into some other branch.
headRef : String
headRef : String

%name PullRequest pr, pr1, pr2

export
Show PullRequest where
show (MkPullRequest number _ _ author state _ headRef) =
show (MkPullRequest number _ _ _ author state _ headRef) =
"[\{show number}] (\{show state}) \{authorString} - headRef: \{headRef}"
where
authorString : String
Expand Down Expand Up @@ -91,17 +93,19 @@ export
parsePR : JSON -> Either String PullRequest
parsePR json =
do pr <- object json
[pullNumber, authorLogin, stateStr, createdAtStr, mergedStr, isDraftStr, reviewerList, head] <- lookupAll ["pull_number", "author", "state", "created_at", "merged", "draft", "reviewers", "head_ref"] pr
number <- integer pullNumber
author <- string authorLogin
merged <- bool mergedStr
isDraft <- bool isDraftStr
state <- (parseState merged) =<< string stateStr
createdAt <- parseDateTime =<< string createdAtStr
reviewers <- array string reviewerList
headRef <- string head
[pullNumber, pullTitle, authorLogin, stateStr, createdAtStr, mergedStr, isDraftStr, reviewerList, head] <- lookupAll ["pull_number", "title", "author", "state", "created_at", "merged", "draft", "reviewers", "head_ref"] pr
number <- integer pullNumber
title <- string pullTitle
author <- string authorLogin
merged <- bool mergedStr
isDraft <- bool isDraftStr
state <- (parseState merged) =<< string stateStr
createdAt <- parseDateTime =<< string createdAtStr
reviewers <- array string reviewerList
headRef <- string head
pure $ MkPullRequest {
number
, title
, createdAt
, isDraft
, author
Expand Down
7 changes: 6 additions & 1 deletion src/Help.idr
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ subcommandHelp' n@"pr" = subcommand n [argument False "--draft", argument False
[ reflow "Identify an existing PR or create a new one for the current branch."
, reflow "Optionally apply any number of labels by prefixing them with '#'."
]
subcommandHelp' n@"contribute" = subcommand n [argument False "-c/--checkout", argument False "-<num>", argument False "-i/--ignore {<uri>/<pr-number>}"]
subcommandHelp' n@"contribute" = subcommand n [argument False "-c/--checkout | -l/--list", argument False "-<num>", argument False "-i/--ignore {<uri>/<pr-number>}"]
[ reflow """
Contribute to an open PR. Prints a URL. Prioritizes PRs you are
requested to review but will also return other PRs.
Expand All @@ -88,6 +88,11 @@ subcommandHelp' n@"contribute" = subcommand n [argument False "-c/--checkout", a
PR. Do this with the '--ignore' option followed by a GitHub URI or
Pull Request number.
"""
, reflow """
Use --list to list multiple PRs instead of just one. This option
supports skipping PRs but does not support the checkout option because
there is no single PR that should be checked out.
"""
]
subcommandHelp' n@"graph" = subcommand n [argument False "-c/--completed", argument True "<team-slug>"]
[reflow "Graph the relative review workload of the members of the given GitHub Team."]
Expand Down
5 changes: 3 additions & 2 deletions support/js/okit.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ const digPr = pr => {
draft: pr.draft,
merged: pr.merged_at !== null,
reviewers: pr.requested_reviewers.map(u => u.login),
head_ref: pr.head.ref
head_ref: pr.head.ref,
title: pr.title
}
}
const digPrs = prJson =>
Expand Down Expand Up @@ -129,7 +130,7 @@ const okit_list_reviewers = (octokit, owner, repo, state, per_page, onSuccess, o
)

// list PRs
// Executes callback with [{ "pull_number": Int, "author": String, "state": String, "merged": Boolean, "reviewers": [String] }]
// Executes callback with [{ "pull_number": Int, "author": String, "state": String, "merged": Boolean, "reviewers": [String], "title": String }]
const okit_list_pull_requests = (octokit, owner, repo, state, per_page, page, onSuccess, onFailure) =>
idris__okit_unpromisify(
octokit.rest.pulls.list({ owner, repo, state, per_page, page }),
Expand Down
6 changes: 5 additions & 1 deletion test/expected_help.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Subcommands:
properties: requestTeams, requestUsers, commentOnRequest, defaultRemote,
mainBranch, theme, githubPAT, assignUsers, assignTeams,
commentOnAssign
contribute [-c/--checkout] [-<num>] [-i/--ignore {<uri>/<pr-number>}]
contribute [-c/--checkout | -l/--list] [-<num>] [-i/--ignore {<uri>/<pr-number>}]
Contribute to an open PR. Prints a URL. Prioritizes PRs you are requested
to review but will also return other PRs.

Expand All @@ -22,6 +22,10 @@ Subcommands:
machine) if you would like to more permanently skip a potential PR. Do
this with the '--ignore' option followed by a GitHub URI or Pull Request
number.

Use --list to list multiple PRs instead of just one. This option supports
skipping PRs but does not support the checkout option because there is no
single PR that should be checked out.
graph [-c/--completed] {<team-slug>}
Graph the relative review workload of the members of the given GitHub
Team.
Expand Down
Loading