Merged
Conversation
2 tasks
sujaypatil96
approved these changes
Nov 24, 2021
Contributor
sujaypatil96
left a comment
There was a problem hiding this comment.
@gaurav: Thank you so much for this PR. I've left a few trivial comments, which you can either choose to address or not. But everything works seamlessly even without them, so you can merge whenever.🚀
turbomam
reviewed
Nov 30, 2021
Member
turbomam
left a comment
There was a problem hiding this comment.
I was able to run pytest and flake8 locally, so I'm inclined to approve this if the GH actions work
This seems unnecessary: even if we hit the cache, the versions of the files in the Poetry config and lock files might have changed, and so require re-installation.
Member
|
Thanks for explaining, Gaurav
… On Dec 9, 2021, at 2:53 PM, Gaurav Vaidya ***@***.***> wrote:
@gaurav commented on this pull request.
In .github/workflows/pytest.yaml <#41 (comment)>:
> + - id: cache-poetry
uses: ***@***.***
with:
- path: ~/.local/share/virtualenvs
- key: ${{ runner.os }}-pipenv-${{ hashFiles('**/Pipfile.lock') }}
- - name: Install dependencies
- if: steps.cache-pipenv.outputs.cache-hit != 'true'
- run: |
- pipenv install --deploy --dev
+ path: ~/.local
+ key: ${{ runner.os }}-python-${{ matrix.python-version }}-poetry-${{ hashFiles('**/poetry.lock') }}
Sujay: Yup, that makes sense! Fixed in 41522b9 <41522b9>.
Mark:
Is this PR primarily about getting poetry to work with the GH actions? Is the ability to use poetry in a locally cloned repo a direct side effect of that? A new user would still have to install the poetry application and run the poetry install command before doing any local development or testing. right?
It's primarily about getting Poetry settings in here so that you can run the local scripts and tests and things with poetry run pytest ..., but one way of testing that we've set that up correctly is that to ensure that GitHub Actions can run the tests via Poetry as well.
This caching is allows us to keep a copy of the installed dependencies on the GitHub Actions system <https://github.com/snok/install-poetry#caching-the-poetry-installation> -- that way, we don't need to redownload the dependencies every time we run the tests.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub <#41 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGAEX6OD7ZK2SX23QYJA5RTUQECLVANCNFSM5IKQSIMQ>.
|
This file contains hidden or 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
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.
This PR replaces Pipenv with Poetry, which is better and faster at managing dependencies.