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-569 get rid of build time env variables #272

Merged
merged 5 commits into from Jul 3, 2018

Conversation

Projects
None yet
3 participants
@tadovas
Copy link
Member

commented Jun 27, 2018

…ined networks with posibility to override values one by one

@tadovas tadovas force-pushed the feature/MYST-569-get-rid-of-build-time-env branch from 07a4559 to d268500 Jun 28, 2018


// TODO this function can be aligned with server function when client and server options will merge into
func getNetworkDefinition(options CommandOptions) metadata.NetworkDefinition {
network := metadata.TestNetworkDefinition

This comment has been minimized.

Copy link
@zolia

zolia Jun 28, 2018

Member

I would use 'TestNetDefinition', since we talk about specific network name 'TestNet'

This comment has been minimized.

Copy link
@tadovas

tadovas Jun 28, 2018

Author Member

Fixed

@zolia

This comment has been minimized.

Copy link
Member

commented Jun 28, 2018

other stuff looks good.

@@ -91,7 +95,7 @@ func NewCommandWith(
natService: natService,
dialogWaiterFactory: func(myID identity.Identity) communication.DialogWaiter {
return nats_dialog.NewDialogWaiter(
nats_discovery.NewAddressGenerate(options.BrokerAddress, myID),
nats_discovery.NewAddressGenerate(networkDefintion.BrokerAddress, myID),

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 28, 2018

Member

networkDefintion -> networkDefinition

This comment has been minimized.

Copy link
@tadovas

tadovas Jun 28, 2018

Author Member

fixed

}

// TestNetworkDefinition defines parameters for test network (currently default network)
var TestNetworkDefinition = NetworkDefinition{

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 28, 2018

Member

TestNetwork.. -< Testnet..

This comment has been minimized.

Copy link
@tadovas

tadovas Jun 28, 2018

Author Member

fixed

// MysteriumAPIURL stores MYSTERIUM_API_URL env variable that is complied in
var MysteriumAPIURL string
// NetworkDefinition structure holds all parameters which describe particular network
type NetworkDefinition struct {

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 28, 2018

Member

+1 kudos

@@ -125,13 +122,14 @@ func ParseArguments(args []string) (options CommandOptions, err error) {
flags.StringVar(
&options.DiscoveryAPIAddress,
"discovery-address",
cmd.MysteriumAPIURL,
"",

This comment has been minimized.

Copy link
@Waldz

Waldz Jun 28, 2018

Member

Would be nice to print current defaults as was before

This comment has been minimized.

Copy link
@tadovas

tadovas Jun 28, 2018

Author Member

There are no defaults anymore

This comment has been minimized.

Copy link
@Waldz

Waldz Jul 2, 2018

Member

There are, DEVNET is default, I want to show it clearly

This comment has been minimized.

Copy link
@tadovas

tadovas Jul 2, 2018

Author Member

Testnet is default as defined in network.go there are no default override values.

This comment has been minimized.

Copy link
@Waldz

Waldz Jul 2, 2018

Member

We already started to use terminology Testnet/Devnet or TESTNET/DEVNET in communication, event You in previous sentence

This comment has been minimized.

Copy link
@tadovas

tadovas Jul 2, 2018

Author Member

Ok I will stick to "Testnet" "Devnet" naming

Got rid of env build files, metadata package now will hold all predef…
…ined networks with posibility to override values one by one

@tadovas tadovas force-pushed the feature/MYST-569-get-rid-of-build-time-env branch from d268500 to 9e93e9f Jun 28, 2018

tadovas added some commits Jun 28, 2018

@tadovas tadovas force-pushed the feature/MYST-569-get-rid-of-build-time-env branch from 9e93e9f to 3a6fe5f Jun 28, 2018

network := metadata.TestNetDefinition

switch {
case options.LocalNet:

This comment has been minimized.

Copy link
@Waldz

Waldz Jul 2, 2018

Member

Why not to have default in switch statement

This comment has been minimized.

Copy link
@tadovas

tadovas Jul 2, 2018

Author Member

you will need bigger scope variable anyway, so either it can be left empty and initialized in default: of switch or it can be initialized right away and let the switch to do modification if needed. I chose the second approach

Added scripts for localnet setup/teardown (setup.sh and teardown.sh),…
… e2e reuses localnet environment for tests

@tadovas tadovas force-pushed the feature/MYST-569-get-rid-of-build-time-env branch from a7b6479 to d571cf2 Jul 2, 2018

@Waldz

Waldz approved these changes Jul 2, 2018

@zolia

zolia approved these changes Jul 3, 2018

@tadovas tadovas merged commit e7c3097 into master Jul 3, 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

@tadovas tadovas deleted the feature/MYST-569-get-rid-of-build-time-env branch Jul 3, 2018

@Arrnas Arrnas referenced this pull request Jul 15, 2018

Closed

Improve documentation #297

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.