Extract code owner-required changes for workflow files #807
Conversation
evandrosaturnino
requested review from
pdyraga,
lukasz-zimnoch and
cygnusv
as code owners
|
Preview uploaded to https://preview.dashboard.test.threshold.network/update-code-owner-files/index.html. |
|
Preview uploaded to https://preview.dashboard.test.threshold.network/update-code-owner-files/index.html. |
.github/workflows/dashboard-ci.yml
Outdated
| @@ -152,14 +152,14 @@ jobs: | |||
| gcpBucketName: dashboard.test.threshold.network | |||
| preview: false | |||
| secrets: | |||
| ethUrlHttp: ${{ secrets.SEPOLIA_ETH_HOSTNAME_HTTP }} | |||
| ethUrlWS: ${{ secrets.SEPOLIA_ETH_HOSTNAME_WS }} | |||
| alchemyApi: ${{ secrets.ALCHEMY_API }} | |||
There was a problem hiding this comment.
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.
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
| 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.
Do we need to specify those for the mainnet workflow? Is it because they are used from reusable-build-and-publish.yml? Would be good to have this decision captured in the commit message.
There was a problem hiding this comment.
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
And also reflected in the app in the PR #809
There was a problem hiding this comment.
Following this same approach I also updated the subgraph variable updating the name with the key suffix: update subgraph api key variable
|
Preview uploaded to https://preview.dashboard.test.threshold.network/update-code-owner-files/index.html. |
|
Preview uploaded to https://preview.dashboard.test.threshold.network/update-code-owner-files/index.html. |
| description: Protocol used by the Testnet Electrum server. | ||
| required: true | ||
| electrumHost: | ||
| description: Host pointing to the Electrum server. | ||
| description: Host pointing to the Testnet Electrum server. | ||
| required: true | ||
| electrumPort: | ||
| description: Port the Electrum server listens on. | ||
| description: Port the Testnet Electrum server listens on. |
There was a problem hiding this comment.
It is not necessarily testnet, right?
There was a problem hiding this comment.
In the context of reusable-build-and-publish.yml, the workflow is expected to receive only testnet values for these secret variables. However, in a broader context, electrum variables can be passed in different ways (e.g., through other workflow files).
To avoid any potential confusion, I have removed this term in: remove testnet term of descriptions in reusable workflow
|
Preview uploaded to https://preview.dashboard.test.threshold.network/update-code-owner-files/index.html. |
|
Preview uploaded to https://preview.dashboard.test.threshold.network/update-code-owner-files/index.html. |
This PR extracts the changes for the github workflow files that require code owner approval from the larger PR (#785):
Why this change?
The original PR touched 200 files, and to streamline the review process and ensure that code owner reviews are conducted efficiently, these files have been isolated into a separate PR.
What’s included?
Next Steps:
Please let me know if you have any questions or require additional context.