10000 [#114] Add non-interactive mode for `summoner` by vrom911 · Pull Request #421 · kowainik/summoner · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[#114] Add non-interactive mode for summoner #421

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 5 commits into from
Feb 23, 2020

Conversation

vrom911
Copy link
Member
@vrom911 vrom911 commented Feb 23, 2020

Resolves #114

@vrom911 vrom911 added CLI Command line interface interaction enhancement labels Feb 23, 2020
@vrom911 vrom911 added this to the v2.0: Major update milestone Feb 23, 2020
@vrom911 vrom911 requested a review from chshersh as a code owner February 23, 2020 16:33
@vrom911 vrom911 self-assigned this Feb 23, 2020
Copy link
Contributor
@hint-man hint-man bot left a comment

Choose a reason for hiding this comment

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

Do you know why your PR is still not approved? Because I chose not to approve it. But they will.

@vrom911 vrom911 force-pushed the vrom911/114-Add-non-interactive-mode-for-summoner branch from 523bb1c to 6beac3a Compare February 23, 2020 16:42
Copy link
Contributor
@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Extremely nice refactoring! I like how a lot of functions were moved into the library and reused 👍

Comment on lines +469 to +473
libraryP d = flag Idk d $ mconcat
[ long "library"
, short 'l'
, help "Library folder"
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice refactoring with CLI options!

Comment on lines +18 to +28
-- | Switcher for non-interactive mode.
data Interactivity
= Interactive
| NonInteractive
deriving stock (Show, Eq)

-- | Is interactivity mode 'NonInteractive'?
isNonInteractive :: Interactivity -> Bool
isNonInteractive = \case
NonInteractive -> True
Interactive -> False
Copy link
Contributor

Choose a reason for hiding this comment

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

I already like this module 😍 Nice idea to use enums instead of booleans 👍

could be.
-}

module Summoner.Mode
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also reexport this module from Summoner.hs

Comment on lines 309 to 314
year <- currentYear
guessConfig <- guessConfigFromGit
lc <- fetchLicenseCustom
licenseName
(fromMaybe "YOUR NAME" $ getLast $ cFullName guessConfig)
year
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these lines can be moved into a separate function that returns IO License. And then you can reuse them in both CLI and TUI files.

vrom911 and others added 2 commits February 23, 2020 17:03
Co-Authored-By: Dmitrii Kovanikov <kovanikov@gmail.com>
Copy link
Contributor
@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Amazing!

@chshersh chshersh merged commit 5074253 into master Feb 23, 2020
@chshersh chshersh deleted the vrom911/114-Add-non-interactive-mode-for-summoner branch February 23, 2020 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Command line interface interaction enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add non-interactive mode for summoner
2 participants
0