Run compatibility test only in main branch.#397
Conversation
2c5bf88 to
fec6c21
Compare
Moved the compatibility check into it's own workflow. The workflow uses a flag to specify whether the step is enabled, as it cannot just be skipped, due to the all-jobs-complete dependency on the compatibility-test.
fec6c21 to
e796473
Compare
|
Workflow error here: https://github.com/MetaMask/eth-json-rpc-middleware/actions/runs/17095017345 |
.github/workflows/main.yml
Outdated
| name: Compatibility test | ||
| uses: ./.github/workflows/compatibility-test.yml | ||
| with: | ||
| enabled: ${{ github.ref == 'refs/heads/main' }} |
There was a problem hiding this comment.
Interesting idea, but maybe it would be simpler to skip the job here rather than using a parameter to skip inside the compatibility-test workflow?
There was a problem hiding this comment.
Since the job is now added to all-jobs-completed would skipping the job here instead of in the workflow cause all-jobs-completed to fail if not on main?
There was a problem hiding this comment.
Well, since this is an optional job, it doesn't need to be listed as a dependency of all-jobs-completed necessarily.
| enabled-check: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Skipping compatibility test | ||
| if: !inputs.enabled | ||
| run: echo "Skipping compatibility test" | ||
|
|
There was a problem hiding this comment.
Should we remove this?
| enabled-check: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Skipping compatibility test | |
| if: !inputs.enabled | |
| run: echo "Skipping compatibility test" |
.github/workflows/main.yml
Outdated
| with: | ||
| enabled: ${{ github.ref == 'refs/heads/main' }} |
There was a problem hiding this comment.
Should we replace this with the following?
| with: | |
| enabled: ${{ github.ref == 'refs/heads/main' }} | |
| if: ${{ github.ref == 'refs/heads/main' }} |
.github/workflows/main.yml
Outdated
| - check-workflows | ||
| - analyse-code | ||
| - build-lint-test | ||
| - compatibility-test |
There was a problem hiding this comment.
Should we remove this?
| - compatibility-test |
| inputs: | ||
| enabled: | ||
| required: false | ||
| type: boolean | ||
| default: true | ||
|
|
There was a problem hiding this comment.
Should we remove these lines?
| inputs: | |
| enabled: | |
| required: false | |
| type: boolean | |
| default: true |
| needs: enabled-check | ||
| if: inputs.enabled |
There was a problem hiding this comment.
Should we remove these lines?
| needs: enabled-check | |
| if: inputs.enabled |
…sulting simplifications
| name: Compatibility test | ||
| uses: ./.github/workflows/compatibility-test.yml | ||
| if: ${{ github.ref == 'refs/heads/main' }} | ||
|
|
There was a problem hiding this comment.
Bug: Compatibility Test Not Blocking Main Branch
The new compatibility-test job isn't listed as a dependency for all-jobs-completed. This means compatibility-test failures on the main branch won't block the overall workflow from succeeding, potentially allowing problematic releases.
Adds methods `wallet_requestExecutionPermissions` and `wallet_revokeExecutionPermission`, as defined in this revision of the EIP-7715 specification ethereum/ERCs#1098. This supports Readable Permissions project, and is related to the following PRs: - MetaMask/smart-accounts-kit#60 - MetaMask/metamask-extension#35193 Note: workflows are failing due to existing problems, fixed by #397
This is the release for version 17.1.0. ``` a10c7fb feat: add RPC methods described in (revised) EIP-7715 (#396) 45ed998 Run compatibility test only in main branch. (#397) ``` --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Jeff Smale <6363749+jeffsmale90@users.noreply.github.com> Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
Adds methods `wallet_requestExecutionPermissions` and `wallet_revokeExecutionPermission`, as defined in this revision of the EIP-7715 specification ethereum/ERCs#1098. This supports Readable Permissions project, and is related to the following PRs: - MetaMask/smart-accounts-kit#60 - MetaMask/metamask-extension#35193 Note: workflows are failing due to existing problems, fixed by MetaMask/eth-json-rpc-middleware#397
Moved the compatibility test into it's own workflow.
This implements the change described here MetaMask/metamask-module-template#269, ensuring that the compatibility test continues to provide value in identifying issues, but does not hold up individual PRs that do not introduce these issues.
The workflow uses a flag to specify whether the compatibility test is enabled, keeping the branching logic in the main workflow file. The compatibility-test.yml workflow cannot be a single job withif: inputs.enabled, because this would mark the workflow as skipped, causing theall-jobs-completejob to also be marked as skipped.all-jobs-completedoes not depend oncompatibility-testsocompatibililty-testcan simply be skipped in non-main branches.