Skip to content
This repository was archived by the owner on May 28, 2022. It is now read-only.

nits: modified monitor to handle new environments#68

Open
yourbuddyconner wants to merge 13 commits intomainfrom
conner/script-changes
Open

nits: modified monitor to handle new environments#68
yourbuddyconner wants to merge 13 commits intomainfrom
conner/script-changes

Conversation

@yourbuddyconner
Copy link
Copy Markdown
Contributor

No description provided.


this.origin = origin;
this.remotes = getNetworks().filter((m) => m != origin);
this.remotes = getReplicas(origin)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

run prettier pls

return networks;
}

function getReplicas(origin: string) {
Copy link
Copy Markdown
Member

@anna-carroll anna-carroll Jan 22, 2022

Choose a reason for hiding this comment

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

this is gonna be a PITA to maintain as we add more chains.

i would suggest something like

const hub = HUB[environment];
const domains = DOMAINS[environment];
if (origin === hub) { 
   return domains.filter(d => d !== hub); 
} else { 
   return [hub]; 
}

then you only ever need to add new domains to DOMAINS.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

would even be cleaner to have some better way than hard-coding HUB and DOMAINS somewhere, but better than hard-coding everything imo

@anna-carroll anna-carroll changed the title nits: modified scripts to handle new environments nits: modified monitor to handle new environments Jan 26, 2022
@yourbuddyconner yourbuddyconner requested a review from Imti as a code owner February 3, 2022 01:44
@arnaud036 arnaud036 force-pushed the conner/script-changes branch from d054d1a to 593899c Compare March 2, 2022 00:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants