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

Ensure configured wrapper port is exposed to Aragon client #201

Merged
merged 1 commit into from Sep 25, 2018

Conversation

Projects
None yet
2 participants
@PascalPrecht
Copy link
Contributor

PascalPrecht commented Sep 11, 2018

This needs to be rebased on top of #200 once it lands

@PascalPrecht

This comment has been minimized.

Copy link
Contributor

PascalPrecht commented Sep 12, 2018

Only the second commit needs to be reviewed here.

env: {
REACT_APP_ENS_REGISTRY_ADDRESS: ctx.ens,
BROWSER: 'none',
ARAGON_CLIENT_PORT: WRAPPER_PORT

This comment has been minimized.

@PascalPrecht

PascalPrecht Sep 12, 2018

Contributor

This needs to be renamed to REACT_APP_PORT as per aragon/aragon#352 (comment)

@PascalPrecht PascalPrecht force-pushed the PascalPrecht:fix/use-wrapper-port branch 2 times, most recently from 8981c85 to b05b316 Sep 12, 2018

env: {
REACT_APP_ENS_REGISTRY_ADDRESS: ctx.ens,
BROWSER: 'none',
REACT_APP_PORT: WRAPPER_PORT

This comment has been minimized.

@izqui

izqui Sep 14, 2018

Member

Can be rebased now to use the value provided in the --client-port flag!

fix(commands/run): ensure configured client wrapper port is used
Prior to this commit, using `WRAPPER_PORT` and changing it to a users needs
didn't have any effect as

a) the port has never been passed down to the Aragon client
b) Aragon client's script isn't prepared to retrieve values from the environment

This commit ensures that the port is set as an environment variable,
enabling Aragon client to take advantage of it.

Fixes #198

@PascalPrecht PascalPrecht force-pushed the PascalPrecht:fix/use-wrapper-port branch from b05b316 to 7af356a Sep 14, 2018

@PascalPrecht

This comment has been minimized.

Copy link
Contributor

PascalPrecht commented Sep 14, 2018

Rebased @izqui

@izqui izqui merged commit 20a07a3 into aragon:master Sep 25, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
license/cla Contributor License Agreement is signed.
Details

@PascalPrecht PascalPrecht deleted the PascalPrecht:fix/use-wrapper-port branch Sep 25, 2018

galactusss added a commit to galactusss/aragon-cli that referenced this pull request Jan 5, 2019

fix(commands/run): ensure configured client wrapper port is used (ara…
…gon#201)

Prior to this commit, using `WRAPPER_PORT` and changing it to a users needs
didn't have any effect as

a) the port has never been passed down to the Aragon client
b) Aragon client's script isn't prepared to retrieve values from the environment

This commit ensures that the port is set as an environment variable,
enabling Aragon client to take advantage of it.

Fixes aragon#198
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment