feat: Skip running tests if there are no source code changes [APE-1094] #1602
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
antazoey
reviewed
Aug 22, 2023
src/ape/api/accounts.py
Outdated
| @@ -215,6 +221,11 @@ def deploy( | |||
| :class:`~ape.contracts.ContractInstance`: An instance of the deployed contract. | |||
| """ | |||
| txn = contract(*args, **kwargs) | |||
| if kwargs.get("value") and not contract.contract_type.constructor.is_payable: | |||
There was a problem hiding this comment.
you may want to put this change on a separate branch
.github/workflows/test.yaml
Outdated
| - name: Run flake8 | ||
| if: steps.detect-changes.outputs.files_changed == 'true' | ||
| run: flake8 . | ||
| - name: Run mdformat |
There was a problem hiding this comment.
mdformat can always run, that is probably easier.
because even if you change the README or this file (test.yaml), mdformat may have something to do
.github/workflows/test.yaml
Outdated
| - name: Run mdformat | ||
| if: steps.detect-changes.outputs.files_changed == 'true' | ||
| run: mdformat . --check | ||
|
|
.github/workflows/test.yaml
Outdated
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Install changed-files action | ||
| run: npm install umani/changed-files |
There was a problem hiding this comment.
is there a python version of this tool? That would fit better and not require installing npm
.github/workflows/test.yaml
Outdated
| - uses: actions/checkout@v3 | ||
|
|
||
| - name: Setup Python | ||
| uses: actions/setup-python@v2 |
There was a problem hiding this comment.
Suggested change
| uses: actions/setup-python@v2 | |
| uses: actions/setup-python@v4 |
.github/workflows/test.yaml
Outdated
| if: steps.detect-changes.outputs.files_changed == 'true' | ||
| run: mypy . | ||
|
|
||
| functional: |
There was a problem hiding this comment.
can we skip this whole section if there are no changes? Like there isn't a need to install Go or Geth if there are no changes
antazoey
reviewed
Aug 22, 2023
|
the branchI created is out of date. I will fix it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I did
I tried to follow this guide https://github.com/umani/changed-files#readme. Test skipping was successful on my fork. But I wouldn't consider merging this PR just not yet. I'm no workflow expert. Hoping someone will eventually look at this PR & improve it.
fixes: #1488
How I did it
How to verify it
Checklist