-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat: update deploy script #33
base: develop
Are you sure you want to change the base?
Conversation
…gin repo address from the env vars
…ommons config or the env file
let pluginRepoFactoryAddress; | ||
let subdomainRegistrar; | ||
if ( | ||
process.env.PLUGIN_REPO_FACTORY_ADDRESS && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think instead of 2 lines, which is still not trustful, it's better to directly check if process.env.PLUGIN_REPO_FACTORY_ADDRESS is a type of address. This will eliminate 2 checks.
These 2 checks are not trustworthy because if i set PLUGIN_REPO_FACTORY_ADDRESS=blax, this will still work which is wrong.
const subdomain = | ||
subdomainRegistrar !== ethers.constants.AddressZero | ||
? PLUGIN_REPO_ENS_SUBDOMAIN_NAME | ||
: 'empty'; // todo set to empty when is possible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why empty
? even if you make it equal to '', it will still work. no ?
Description
Please include a summary of the change and be sure you follow the contributions rules we do provide here
Task ID: OS-?
Type of change
See the framework lifecylce in
packages/contracts/docs/framework-lifecycle
to decide what kind of change this pull request is.Checklist:
CHANGELOG.md
file in the root folder.DEPLOYMENT_CHECKLIST
file in the root folder.UPDATE_CHECKLIST
file in the root folder.