-
Notifications
You must be signed in to change notification settings - Fork 28
Extract code owner-required changes for workflow files #807
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
Changes from 2 commits
67c3c0d
3c78bf3
52522e7
c2e6c90
d333351
9390a87
4ee32ce
15dc6da
46602fc
f43d879
ecf1070
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,16 +49,19 @@ jobs: | |
env: | ||
PUBLIC_URL: /${{ github.ref_name }} | ||
CHAIN_ID: 1 | ||
ETH_HOSTNAME_HTTP: ${{ secrets.MAINNET_ETH_HOSTNAME_HTTP }} | ||
ETH_HOSTNAME_WS: ${{ secrets.MAINNET_ETH_HOSTNAME_WS }} | ||
ALCHEMY_API: ${{ secrets.ALCHEMY_API }} | ||
NODE_OPTIONS: --max_old_space_size=4096 | ||
ELECTRUM_PROTOCOL: ${{ secrets.MAINNET_ELECTRUMX_PROTOCOL }} | ||
ELECTRUM_HOST: ${{ secrets.MAINNET_ELECTRUMX_HOST }} | ||
ELECTRUM_PORT: ${{ secrets.MAINNET_ELECTRUMX_PORT }} | ||
MAINNET_ELECTRUM_PROTOCOL: ${{ secrets.MAINNET_ELECTRUMX_PROTOCOL }} | ||
MAINNET_ELECTRUM_HOST: ${{ secrets.MAINNET_ELECTRUMX_HOST }} | ||
MAINNET_ELECTRUM_PORT: ${{ secrets.MAINNET_ELECTRUMX_PORT }} | ||
TESTNET_ELECTRUM_PROTOCOL: ${{ secrets.TESTNET_ELECTRUMX_PROTOCOL }} | ||
TESTNET_ELECTRUM_HOST: ${{ secrets.TESTNET_ELECTRUMX_HOST }} | ||
TESTNET_ELECTRUM_PORT: ${{ secrets.TESTNET_ELECTRUMX_PORT }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to specify those for the mainnet workflow? Is it because they are used from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although the multiple networks app accepts both mainnet and testnet chains at the same time, and we initially planned to launch the production app with both, in the last days we've decided to move forward with launching the mainnet dashboard with only mainnet L1 and L2. Therefore, it makes sense to remove these env variables from the dashboard-mainnet.yml, as you pointed out. This is being addressed in: consolidate mainnet and testnet electrum variables and remove testnet electrum variable in reusable workflow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following this same approach I also updated the subgraph variable updating the name with the key suffix: update subgraph api key variable |
||
SENTRY_SUPPORT: true | ||
SENTRY_DSN: ${{ secrets.MAINNET_SENTRY_DSN }} | ||
TRM_SUPPORT: ${{ secrets.TRM_SUPPORT }} | ||
WALLET_CONNECT_PROJECT_ID: ${{ secrets.WALLET_CONNECT_PROJECT_ID }} | ||
TBTC_SUBGRAPH_API: ${{ secrets.TBTC_SUBGRAPH_API }} | ||
GOOGLE_TAG_MANAGER_SUPPORT: false | ||
GOOGLE_TAG_MANAGER_ID: ${{ secrets.GOOGLE_TAG_MANAGER_ID }} | ||
|
||
|
@@ -68,19 +71,22 @@ jobs: | |
env: | ||
PUBLIC_URL: / | ||
CHAIN_ID: 1 | ||
ETH_HOSTNAME_HTTP: ${{ secrets.MAINNET_ETH_HOSTNAME_HTTP }} | ||
ETH_HOSTNAME_WS: ${{ secrets.MAINNET_ETH_HOSTNAME_WS }} | ||
ALCHEMY_API: ${{ secrets.ALCHEMY_API }} | ||
NODE_OPTIONS: --max_old_space_size=4096 | ||
POSTHOG_SUPPORT: true | ||
POSTHOG_API_KEY: ${{ secrets.MAINNET_POSTHOG_API_KEY }} | ||
POSTHOG_HOSTNAME_HTTP: ${{ secrets.MAINNET_POSTHOG_HOSTNAME_HTTP }} | ||
ELECTRUM_PROTOCOL: ${{ secrets.MAINNET_ELECTRUMX_PROTOCOL }} | ||
ELECTRUM_HOST: ${{ secrets.MAINNET_ELECTRUMX_HOST }} | ||
ELECTRUM_PORT: ${{ secrets.MAINNET_ELECTRUMX_PORT }} | ||
MAINNET_ELECTRUM_PROTOCOL: ${{ secrets.MAINNET_ELECTRUMX_PROTOCOL }} | ||
MAINNET_ELECTRUM_HOST: ${{ secrets.MAINNET_ELECTRUMX_HOST }} | ||
MAINNET_ELECTRUM_PORT: ${{ secrets.MAINNET_ELECTRUMX_PORT }} | ||
TESTNET_ELECTRUM_PROTOCOL: ${{ secrets.TESTNET_ELECTRUMX_PROTOCOL }} | ||
TESTNET_ELECTRUM_HOST: ${{ secrets.TESTNET_ELECTRUMX_HOST }} | ||
TESTNET_ELECTRUM_PORT: ${{ secrets.TESTNET_ELECTRUMX_PORT }} | ||
SENTRY_SUPPORT: true | ||
SENTRY_DSN: ${{ secrets.MAINNET_SENTRY_DSN }} | ||
TRM_SUPPORT: ${{ secrets.TRM_SUPPORT }} | ||
WALLET_CONNECT_PROJECT_ID: ${{ secrets.WALLET_CONNECT_PROJECT_ID }} | ||
TBTC_SUBGRAPH_API: ${{ secrets.TBTC_SUBGRAPH_API }} | ||
GOOGLE_TAG_MANAGER_SUPPORT: true | ||
GOOGLE_TAG_MANAGER_ID: ${{ secrets.GOOGLE_TAG_MANAGER_ID }} | ||
|
||
|
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.
Is this an HTTP or WS URL or maybe an API key? The previous version was more specific, and it was clear what value should be provided. I would rename it to
ALCHEMY_API_KEY
or something more specific, depending on what this value should represent.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.
Agreed. This is being addressed in the following commit: update alchemy api key env variable name
And also reflected in the app in the PR #809