-
Notifications
You must be signed in to change notification settings - Fork 16
Add liquidity-pools as dependency #6
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 all commits
f579931
a1b022f
87d1090
8c88378
8f1a42b
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 |
|---|---|---|
|
|
@@ -82,54 +82,54 @@ jobs: | |
| env: | ||
| FOUNDRY_PROFILE: ci | ||
|
|
||
| coverage: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| # coverage: | ||
| # runs-on: ubuntu-latest | ||
| # permissions: | ||
| # contents: read | ||
| # pull-requests: write | ||
| # steps: | ||
| # - uses: actions/checkout@v3 | ||
|
|
||
| - name: Install Foundry | ||
| uses: foundry-rs/foundry-toolchain@v1 | ||
| # - name: Install Foundry | ||
| # uses: foundry-rs/foundry-toolchain@v1 | ||
|
|
||
| - name: Run coverage | ||
| run: forge coverage --report summary --report lcov | ||
| env: | ||
| FORK_TESTS: false | ||
| # - name: Run coverage | ||
| # run: forge coverage --report summary --report lcov | ||
| # env: | ||
| # FORK_TESTS: false | ||
|
|
||
| # # To ignore coverage for certain directories modify the paths in this step as needed. The | ||
| # # below default ignores coverage results for the test and script directories. Alternatively, | ||
| # # to include coverage in all directories, comment out this step. Note that because this | ||
| # # filtering applies to the lcov file, the summary table generated in the previous step will | ||
| # # still include all files and directories. | ||
| # # The `--rc lcov_branch_coverage=1` part keeps branch info in the filtered report, since lcov | ||
| # # defaults to removing branch info. | ||
| # - name: Filter directories | ||
| # run: | | ||
| # sudo apt update && sudo apt install -y lcov | ||
| # lcov --remove lcov.info 'test/*' 'script/*' 'src/libraries/*' --output-file lcov.info --rc lcov_branch_coverage=1 --ignore-errors empty --ignore-errors unused | ||
|
|
||
| # # This step posts a detailed coverage report as a comment and deletes previous comments on | ||
| # # each push. The below step is used to fail coverage if the specified coverage threshold is | ||
| # # not met. The below step can post a comment (when it's `github-token` is specified) but it's | ||
| # # not as useful, and this action cannot fail CI based on a minimum coverage threshold, which | ||
| # # is why we use both in this way. | ||
| # - name: Post coverage report | ||
| # if: github.event_name == 'pull_request' # This action fails when ran outside of a pull request. | ||
| # uses: romeovs/[email protected] | ||
| # with: | ||
| # delete-old-comments: true | ||
| # lcov-file: ./lcov.info | ||
| # github-token: ${{ secrets.GITHUB_TOKEN }} # Adds a coverage summary comment to the PR. | ||
|
|
||
| # - name: Verify minimum coverage | ||
| # if: github.event_name == 'pull_request' | ||
| # uses: zgosalvez/github-actions-report-lcov@v2 | ||
| # with: | ||
| # coverage-files: ./lcov.info | ||
| # minimum-coverage: 60 # Set coverage threshold. | ||
|
|
||
| # To ignore coverage for certain directories modify the paths in this step as needed. The | ||
| # below default ignores coverage results for the test and script directories. Alternatively, | ||
| # to include coverage in all directories, comment out this step. Note that because this | ||
| # filtering applies to the lcov file, the summary table generated in the previous step will | ||
| # still include all files and directories. | ||
| # The `--rc lcov_branch_coverage=1` part keeps branch info in the filtered report, since lcov | ||
| # defaults to removing branch info. | ||
| - name: Filter directories | ||
| run: | | ||
| sudo apt update && sudo apt install -y lcov | ||
| lcov --remove lcov.info 'test/*' 'script/*' 'src/libraries/*' --output-file lcov.info --rc lcov_branch_coverage=1 --ignore-errors empty --ignore-errors unused | ||
|
|
||
| # This step posts a detailed coverage report as a comment and deletes previous comments on | ||
| # each push. The below step is used to fail coverage if the specified coverage threshold is | ||
| # not met. The below step can post a comment (when it's `github-token` is specified) but it's | ||
| # not as useful, and this action cannot fail CI based on a minimum coverage threshold, which | ||
| # is why we use both in this way. | ||
| - name: Post coverage report | ||
| if: github.event_name == 'pull_request' # This action fails when ran outside of a pull request. | ||
| uses: romeovs/[email protected] | ||
| with: | ||
| delete-old-comments: true | ||
| lcov-file: ./lcov.info | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} # Adds a coverage summary comment to the PR. | ||
|
|
||
| - name: Verify minimum coverage | ||
| if: github.event_name == 'pull_request' | ||
| uses: zgosalvez/github-actions-report-lcov@v2 | ||
| with: | ||
| coverage-files: ./lcov.info | ||
| minimum-coverage: 60 # Set coverage threshold. | ||
|
|
||
| # slither-analyze: | ||
| # runs-on: "ubuntu-latest" | ||
| # permissions: | ||
|
|
@@ -155,8 +155,8 @@ jobs: | |
| # uses: github/codeql-action/upload-sarif@v2 | ||
| # with: | ||
| # sarif_file: ${{ steps.slither.outputs.sarif }} | ||
|
|
||
| # - name: "Add Slither summary" | ||
| # run: | | ||
| # echo "## Slither result" >> $GITHUB_STEP_SUMMARY | ||
| # echo "✅ Uploaded to GitHub code scanning" >> $GITHUB_STEP_SUMMARY | ||
| # echo "✅ Uploaded to GitHub code scanning" >> $GITHUB_STEP_SUMMARY | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| [submodule "lib/forge-std"] | ||
| path = lib/forge-std | ||
| url = https://github.com/foundry-rs/forge-std | ||
| [submodule "lib/forge-gas-snapshot"] | ||
| path = lib/forge-gas-snapshot | ||
| url = https://github.com/marktoda/forge-gas-snapshot | ||
| [submodule "lib/liquidity-pools"] | ||
| path = lib/liquidity-pools | ||
| url = https://github.com/centrifuge/liquidity-pools | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| @chimera/=lib/liquidity-pools/lib/chimera/src/ | ||
| forge-std/=lib/forge-std/src/ | ||
| forge-gas-snapshot/=lib/forge-gas-snapshot | ||
|
Contributor
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. I think this should still not be removed. Not sure why it would be. @peculiarity any ideas?
Contributor
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. Yeah, this shouldn't be removed @lemunozm .
Contributor
Author
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. Could it be because it's recently archived? https://github.com/marktoda/forge-gas-snapshot
Contributor
Author
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. At least
Contributor
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. Ah cool, it was upstreamed foundry-rs/foundry#8952
Contributor
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. Not sure if that is the reason why, but it does mean we can remove the dependency, and rather just update foundry and use that going forward! In that case, can you also remove the submodule?
Contributor
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. https://github.com/marktoda/forge-gas-snapshot?tab=readme-ov-file - this is deprecated anyways. So probably we can remove it anyways .
Contributor
Author
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. Done. I've also called this again
Contributor
Author
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. If that's not the correct way of do this I can revert it
Contributor
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. The problem comes when manually add the entry in |
||
| liquidity-pools/=lib/liquidity-pools/ | ||
Uh oh!
There was an error while loading. Please reload this page.