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

Use access token #188

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Use access token #188

wants to merge 3 commits into from

Conversation

0xjocke
Copy link
Contributor

@0xjocke 0xjocke commented May 5, 2022

The access-token parameter was not used for the hosted service deployment and used incorrectly for the decentralised deployment (it's camel cased on the settings object)

My first commit resolved the problem. The nested if statement and repetition of build/deploy exec triggered my refactor brain. I created buildAndDeployHosted and deployDecentralized to make it a little easier to reason about, hopefully you agree with that.

I also remove some await for non async functions

0xjocke added 2 commits May 5, 2022 12:03
- Unwind some of the nested if statement by creating functions for buildAndDeployHosted and  deployDecentralized
@0xjocke 0xjocke requested review from dbeal-eth and noahlitvin May 5, 2022 05:11
if (settings.network === 'None') {
// With the old logic, if network was None and deployDecentralized is true we would NOT build and try to deploy to decentralised network.
// I think we dont want to deploy anything when network is None?
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove this comment if my assumption is correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant