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-681 Start having command "mysterium_node" #314

Merged

Conversation

Projects
None yet
3 participants
@Waldz
Copy link
Member

commented Aug 20, 2018

No description provided.

@Waldz Waldz requested review from tadovas, interro, soffokl and zolia Aug 20, 2018

@@ -0,0 +1,5 @@
#!/bin/bash

This comment has been minimized.

Copy link
@soffokl

soffokl Aug 21, 2018

Member

/bin/run and bin/client_run files looks 100% equal. Do we need to keep both?

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 21, 2018

Author Member

We will drop bin/client_* and bin/server_ then single executable will be ready and stable

package main

import (
"fmt"

This comment has been minimized.

Copy link
@soffokl

soffokl Aug 21, 2018

Member

What do you think about start using goimorts for formatting a file? It groups imports into 3 parts: 1 - standard library; 2 - external dependencies; 3 - internal dependencies, which makes much easy to read a list of imports.

This comment has been minimized.

Copy link
@tadovas

tadovas Aug 21, 2018

Member

We are using goimports on daily basis ( at least some of us). There is a task to migrate full code base and add checks to CI

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 21, 2018

Author Member

Fixed imports for my changed files

bin/build Outdated

go install -ldflags="$(get_linker_ldflags)" cmd/mysterium_node/mysterium_node.go
if [ $? -ne 0 ]; then
printf "\e[0;31m%s\e[0m\n" "Compile failed!"

This comment has been minimized.

Copy link
@soffokl

soffokl Aug 21, 2018

Member

There is a bin/helpers/output.sh with a print_error function, that already added for printing colored output.

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 21, 2018

Author Member

Improved

@@ -44,7 +44,7 @@ func ParseFromCmdArgs(flags *flag.FlagSet, options *DirectoryOptions) error {
return err
}

currentDir, err := os.Getwd()
currentDir, err := os.Executable()

This comment has been minimized.

Copy link
@tadovas

tadovas Aug 21, 2018

Member

Why Getwd doesn't fit ?

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 21, 2018

Author Member

I made command runnable from any location in filesystem:

cd mysterium/node
build/client/mysterium_client

This comment has been minimized.

Copy link
@tadovas

tadovas Aug 21, 2018

Member

so basically current dir is a path to executable ?

This comment has been minimized.

Copy link
@Waldz

Waldz Aug 21, 2018

Author Member

Yes, exactly

@Waldz Waldz force-pushed the Waldz:feature/MYST-681-single-executable branch 2 times, most recently from b37a44d to 4ed1794 Aug 21, 2018

@tadovas
Copy link
Member

left a comment

LGTM

@Waldz Waldz dismissed stale reviews from tadovas and soffokl via 3fa2915 Aug 21, 2018

@Waldz Waldz force-pushed the Waldz:feature/MYST-681-single-executable branch 3 times, most recently from 3743338 to 115d0c9 Aug 21, 2018

Waldz added some commits Aug 20, 2018

@Waldz Waldz force-pushed the Waldz:feature/MYST-681-single-executable branch from 115d0c9 to 7024412 Aug 21, 2018

@soffokl
Copy link
Member

left a comment

Looks good to me.

@Waldz Waldz merged commit 2627d7e into mysteriumnetwork:master Aug 22, 2018

1 check passed

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

@Waldz Waldz deleted the Waldz:feature/MYST-681-single-executable branch Aug 22, 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.