Skip to content

Commit

Permalink
Merge pull request #66 from mattpolzin/only-teams-option
Browse files Browse the repository at this point in the history
Add `assignUsers` configuration option.
  • Loading branch information
mattpolzin authored Sep 11, 2022
2 parents a46333b + bf48a95 commit 37c7616
Show file tree
Hide file tree
Showing 18 changed files with 231 additions and 103 deletions.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Many operating systems have an `open` command (though the name "open" is not ubi
### PR
Running `harmony pr` with a branch checked out will reach out to GitHub to determine if there is an open PR for that branch. If there is a PR, Harmony will print a URI that can be used to view the PR. IF there is not a PR, Harmony will help you create one.

If you need to create a PR still, you will be prompted for a branch to open the PR against (merge into, eventually), a title for the PR, and a description for the PR. If you have an `EDITOR` environment variable set, Harmony will use that editor to get the PR description from you. If you have a PR template at `.github/PULL_REQUEST_TEMPLATE.md`, Harmony will also preload that into your editor. If you do not have an `EDITOR` environment variable set, you will still be able to enter a description from the command line; PR templates are not supported for this input mode.
If you need to create a PR still, you will be prompted for a branch to open the PR against (merge into, eventually), a title for the PR, and a description for the PR. If you have an `EDITOR` environment variable set, Harmony will use that editor to get the PR description from you. If you have a PR template at `.github/PULL_REQUEST_TEMPLATE.md`, Harmony will also preload that into your editor. If you do not have an `EDITOR` environment variable set, you will still be able to enter a description from the command line but PR templates are only supported when an `EDITOR` is specified.

Many operating systems have an `open` command (though the name "open" is not ubiquitous); this means you can run something like `open $(harmony pr)` to open a web browser to an existing PR for the current branch.

Expand Down Expand Up @@ -156,11 +156,12 @@ Running `harmony list <team>` will list the members of the given GitHub Team.
Running `harmony graph <team>` will graph the relative review workload of each of the members of the given GitHub Team.

### Config
Running `harmony config <property>` read the given configuration property. `harmony config <property> <value>` will set the configuration property.
Running `harmony config <property>` will read the given configuration property. `harmony config <property> <value>` will set the configuration property.

Not all configuration properties can be read/set with this command.
#### Properties
- `assignTeams` -- When picking a reviewer from a team, assign the team as a reviewer as well.
- `assignUsers` -- When assigning a team as a reviewer, pick a user to review as well.
- `commentOnAssign` -- When assigning a reviewer chosen by Harmony, comment on the pull request.
- `defaultRemote` -- When pushing new branches, what remote destination should be used.
- `githubPAT` -- If the `$GITHUB_PAT` environment variable is not set, this Personal Access Token is used to authenticate with GitHub.
Expand Down
2 changes: 1 addition & 1 deletion harmony.ipkg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package harmony
version = 1.1.1
version = 1.2.0
authors = "Mathew Polzin"
license = "MIT"
-- brief =
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@mattpolzin/harmony",
"version": "1.1.1",
"version": "1.2.0",
"publishConfig": {
"access": "public"
},
Expand Down
4 changes: 3 additions & 1 deletion src/AppVersion.idr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
module AppVersion

%default total

export
appVersion : String
appVersion = "1.1.1"
appVersion = "1.2.0"

export
printVersion : HasIO io => io ()
Expand Down
6 changes: 2 additions & 4 deletions src/BashCompletion.idr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import Data.Config
import Data.List
import Data.String

import Debug.Trace

%default total

allRootCmds : List String
Expand Down Expand Up @@ -65,8 +63,8 @@ opts : Config => String -> String -> List String
-- should have already been called).

-- then the config command
opts @{_} "--" "config" = settableProps
opts @{_} partialConfigProp "config" = filter (isPrefixOf partialConfigProp) settableProps
opts @{_} "--" "config" = settablePropNames
opts @{_} partialConfigProp "config" = filter (isPrefixOf partialConfigProp) settablePropNames

-- then list, which only accepts a single team slug:
opts @{config} "--" "list" = config.teamSlugs
Expand Down
114 changes: 70 additions & 44 deletions src/Config.idr
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import FFI.GitHub
import Language.JSON
import System
import System.File
import Util

import Text.PrettyPrint.Prettyprinter
import Text.PrettyPrint.Prettyprinter.Render.Terminal

%default total

Expand Down Expand Up @@ -53,6 +57,24 @@ syncIfOld config =
now <- time
pure $ cast (now - oneDay)

||| Determine if any configuration settings are inconsistent with each other or result in
||| behavior that is likely undesirable.
checkConfigConsistency : Config -> Either (Doc AnsiStyle) ()
checkConfigConsistency config = do
checkAssignSettings config
-- other checks...
where
checkAssignSettings : Config -> Either (Doc AnsiStyle) ()
checkAssignSettings config =
if not (config.assignTeams || config.assignUsers)
then Left $ (annotate (color Yellow) . hsep $ [
"`assignUsers` and `assignTeams` are both False."
, "This means `harmony assign` commands will only ever assign users that are specified with the `+<userlogin>` syntax."
, "More commonly, you want Harmony to at least assign either a team or a user from a team when you say `harmony assign teamname`;"
, "It's suggested to either `harmony config assignUsers true` or `harmony config assignTeams true` (or both)."
]) <+> hardline
else Right ()

record GitRemote where
constructor Remote
org, repo : String
Expand Down Expand Up @@ -81,28 +103,23 @@ parseGitHubURI str = parseHTTPS str <|> parseSSH str
parseSSH : String -> Maybe GitRemote
parseSSH = dropPrefix' "[email protected]:" >=> parseSuffix

propSetters : List (String, (Config -> String -> Maybe Config))
propSetters = [
("assignTeams" , update parseBool (\b => { assignTeams := b }))
, ("commentOnAssign", update parseBool (\b => { commentOnAssign := b }))
, ("defaultRemote" , update Just (\s => { defaultRemote := Just s }))
, ("githubPAT" , update Just (\s => { githubPAT := Just $ hide s }))
]
where
parseBool : String -> Maybe Bool
parseBool x with (toLower x)
_ | "yes" = Just True
_ | "true" = Just True
_ | "no" = Just False
_ | "false" = Just False
_ | _ = Nothing
parseBool : String -> Maybe Bool
parseBool x with (toLower x)
_ | "yes" = Just True
_ | "true" = Just True
_ | "no" = Just False
_ | "false" = Just False
_ | _ = Nothing

update : Functor f => (String -> f a) -> (a -> b -> b) -> b -> String -> f b
update f g c = map (flip g c) . f
update : Functor f => (String -> f a) -> (a -> b -> b) -> b -> String -> f b
update f g c = map (flip g c) . f

namespace PropSettersProperties
propSettersCoveragePrf : Data.Config.settableProps = Builtin.fst <$> Config.propSetters
propSettersCoveragePrf = Refl
propSetter : SettableProp n h -> (Config -> String -> Maybe Config)
propSetter AssignTeams = update parseBool (\b => { assignTeams := b })
propSetter AssignUsers = update parseBool (\b => { assignUsers := b })
propSetter CommentOnAssign = update parseBool (\b => { commentOnAssign := b })
propSetter DefaultRemote = update Just (\s => { defaultRemote := Just s })
propSetter GithubPAT = update Just (\s => { githubPAT := Just $ hide s })

||| Attempt to set a property and value given String representations.
||| After setting, write the config and return the updated result.
Expand All @@ -111,31 +128,34 @@ setConfig : Config =>
(prop : String)
-> (value : String)
-> Promise Config
setConfig @{config} prop value with (lookup prop propSetters)
_ | Nothing = reject "\{prop} cannot be set via `config` command."
_ | (Just set) with (set config value)
_ | Nothing = reject "\{value} is not a valid value for \{prop}."
_ | (Just config') = writeConfig config'

propGetters : List (String, (Config -> String))
propGetters = [
("assignTeams" , show . assignTeams)
, ("commentOnAssign", show . commentOnAssign)
, ("defaultRemote" , maybe "Not set (defaults to \"origin\")" show . defaultRemote)
, ("githubPAT" , maybe "Not set (will use $GITHUB_PAT environment variable)" show . githubPAT)
]

namespace PropGettersProperties
propGetterCoveragePrf : Data.Config.settableProps = Builtin.fst <$> Config.propGetters
propGetterCoveragePrf = Refl
setConfig @{config} prop value with (settablePropNamed prop)
_ | Nothing = reject "\{prop} cannot be set via `config` command."
_ | Just (Evidence _ p) with ((propSetter p) config value)
_ | Nothing = reject "\{value} is not a valid value for \{prop}."
_ | Just config' = do either (renderIO @{config'}) pure (checkConfigConsistency config')
writeConfig config'

propGetter : SettableProp n h -> (Config -> String)
propGetter AssignTeams = show . assignTeams
propGetter AssignUsers = show . assignUsers
propGetter CommentOnAssign = show . commentOnAssign
propGetter DefaultRemote = maybe "Not set (defaults to \"origin\")" show . defaultRemote
propGetter GithubPAT = maybe "Not set (will use $GITHUB_PAT environment variable)" show . githubPAT

export
getConfig : Config =>
(prop : String)
-> Promise String
getConfig @{config} prop with (lookup prop propGetters)
getConfig prop | Nothing = reject "\{prop} cannot get read via `config` command."
getConfig prop | (Just get) = pure $ get config
getConfig @{config} prop with (settablePropNamed prop)
getConfig @{config} prop | Nothing = reject "\{prop} cannot get read via `config` command."
getConfig @{config} prop | (Just (Evidence _ p)) = pure $ (propGetter p) config

export
settablePropsWithHelp : Config => String
settablePropsWithHelp = renderString . vsep $ help <$> settablePropNamesAndHelp
where
help : (String, String) -> Doc AnsiStyle
help (n, h) = (annotate (color Green) $ pretty n) <+> pretty ": \{replicate (longestSettablePropName `minus` (length n)) ' ' ++ h}"

||| Look for "origin" in a list of remote names or else
||| fallback to the first name.
Expand Down Expand Up @@ -187,9 +207,12 @@ createConfig envGithubPAT terminalColors editor = do
putStr "Would you like harmony to comment when it assigns reviewers? [Y/n] "
commentOnAssign <- yesUnlessNo . trim <$> getLine

putStr "Would you like harmony to assign teams in addition to individuals when it assigns reviewers? [Y/n] "
putStr "Would you like harmony to assign teams when it assigns reviewers? [Y/n] "
assignTeams <- yesUnlessNo . trim <$> getLine

putStr "Would you like harmony to assign individual users when it assigns reviewers? [Y/n] "
assignUsers <- yesUnlessNo . trim <$> getLine

_ <- liftIO $ octokit pat
putStrLn "Creating config..."
mainBranch <- getRepoDefaultBranch org repo
Expand All @@ -208,6 +231,7 @@ createConfig envGithubPAT terminalColors editor = do
, defaultRemote
, mainBranch
, assignTeams
, assignUsers
, commentOnAssign
, teamSlugs
, orgMembers
Expand All @@ -217,6 +241,7 @@ createConfig envGithubPAT terminalColors editor = do
ignore $ writeConfig config
putStrLn "Your new configuration is:"
printLn config
either renderIO pure (checkConfigConsistency config)
pure config
where
orIfEmpty : Maybe String -> String -> String
Expand All @@ -225,9 +250,10 @@ createConfig envGithubPAT terminalColors editor = do
orIfEmpty (Just _) x = x

yesUnlessNo : String -> Bool
yesUnlessNo "n" = False
yesUnlessNo "N" = False
yesUnlessNo _ = True
yesUnlessNo answer with (toLower answer)
_ | "n" = False
_ | "no" = False
_ | _ = True

org : Maybe GitRemote -> Maybe String
org = map (.org)
Expand Down
Loading

0 comments on commit 37c7616

Please sign in to comment.