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

MYST-495 Show license on startup with subcommands for more info #257

Merged
merged 19 commits into from Jun 5, 2018

Conversation

Projects
None yet
4 participants
@donce
Copy link
Contributor

commented May 31, 2018

No description provided.

@donce donce requested review from tadovas, Waldz and zolia May 31, 2018

@donce donce force-pushed the feature/MYST-495-startup-license branch from df5e5da to 5c46d3f May 31, 2018

@tadovas
Copy link
Member

left a comment

LGTM

@@ -91,6 +93,20 @@ func ParseArguments(args []string) (options CommandOptions, err error) {
"Run an interactive CLI based Mysterium UI",
)

flags.BoolVar(
&options.Warranty,
"warranty",

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 1, 2018

Member

I suggest:
licence.warranty
licence.conditions

This comment has been minimized.

Copy link
@donce

donce Jun 1, 2018

Author Contributor

Updated.

@@ -53,6 +53,8 @@ const statusConnected = "Connected"

// Run runs CLI interface synchronously, in the same thread while blocking it
func (c *Command) Run() (err error) {
license := cmd.GetStartupLicense("type `show w'", "type `show c'")

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 1, 2018

Member

Looks like show w is wrong recomendation

This comment has been minimized.

Copy link
@donce

donce Jun 1, 2018

Author Contributor

Fixed

@@ -53,6 +53,8 @@ const statusConnected = "Connected"

// Run runs CLI interface synchronously, in the same thread while blocking it
func (c *Command) Run() (err error) {
license := cmd.GetStartupLicense("type `show w'", "type `show c'")

This comment has been minimized.

Copy link
@zolia

zolia Jun 1, 2018

Member

GetStartupLicense() - I don't see it defined. I would guess it should return error (file not found.. etc).

This comment has been minimized.

Copy link
@donce

donce Jun 1, 2018

Author Contributor

It was defined in license.go, but it was too long file to be displayed in PR, so it was collapsed.

Split that file so that code would be separate from long texts.

@@ -392,5 +405,7 @@ func newAutocompleter(tequilapi *tequilapi_client.Client, proposals []tequilapi_
getIdentityOptionList(tequilapi),
),
),
readline.PcItem("warranty"),

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 1, 2018

Member

What about licence prefix here, to be consistent with other places

This comment has been minimized.

Copy link
@donce

donce Jun 4, 2018

Author Contributor

Added nesting, which makes more sense in CLI.

@tadovas
Copy link
Member

left a comment

reLGTM

@@ -151,6 +153,14 @@ func (cmd *Command) Start() error {
return nil
}

func printLicense() {
startupLicense := license.GetStartupLicense(
"run program with '-warranty' option",

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 5, 2018

Member

Still not consistent

CLI bool
CLI bool
Warranty bool
Conditions bool

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 5, 2018

Member

Still not consistent

@donce

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2018

@Waldz fixed

donce added some commits May 29, 2018

donce added some commits Jun 1, 2018

@donce donce force-pushed the feature/MYST-495-startup-license branch from 922badd to d7692a2 Jun 5, 2018

@tadovas

tadovas approved these changes Jun 5, 2018

Copy link
Member

left a comment

LGTM

@zolia

zolia approved these changes Jun 5, 2018

@donce donce merged commit eb04615 into master Jun 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@donce donce deleted the feature/MYST-495-startup-license branch Jun 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.