Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Requirements not found error + fix for error in black#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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
nmenardg-keeper wants to merge5 commits intomarian-code:master
base:master
Choose a base branch
Loading
fromnmenardg-keeper:requirements-not-found-error

Conversation

nmenardg-keeper
Copy link

@nmenardg-keepernmenardg-keeper commentedJun 16, 2022
edited
Loading

setup-python action was searching for the requirements.txt file of the linted python project, which in pipenv projects broke the action
and
updated black to get thisfix:psf/black#2966

@marian-code
Copy link
Owner

Now that I am looking at this, I see a bigger problem.
If we do install requirements.txt than linting with mypy is a lot less usefull since it just ignores missing libraries:missing-imports but that is far from ideal. The checks pass but the user does not know that some may have been silently skipped.

Is see few options here:

  • we have to figure out how to install all dependencies ourselves, maybe add some configurable mypy option where the user could point the action to the requirements file(s). We would also ideally need to take care of other installation tools likepoetry
  • second option is to make the user install everything the package requires. But that also has its problems. What if other actions are separated from this one? We are setting our own python version with @setup-python this will overwrite the setup the user has done if I am not mistaken. The @setup-python could be left on the user but then we have to be prepared to handle some non-standard environments whis can be really cumbersome.
  • just state in the README that this action requires arequirements.txt file to be placed in repo even if it should be empty

I am quite short on time and will look into it later, but you could take a shot at this@nmenardg-keeper if you would like :)
Or just let me hear your toughts on this.

@@ -1,4 +1,4 @@
black==21.11b1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We also need to update README file

@nmenardg-keeper
Copy link
Author

You're making good points here. Thinking about it, here are the options I see:

  1. User provides a command to run for installing dependencies
    1. Very flexible, but kind of dirty
  2. Let the user do the setup beforehand.setup-python and installing dependencies
    1. To be honest I don't understand the problems you mention here.
  3. Tell the user to generate a requirements.txt file beforehand
    1. this is possible withpipenv requirements, I don't know about other tools like poetry
    2. Very easy on our part
    3. I don't like the idea of asking people to provide an empty requirements.txt that wouldn't help mypy in the end

I'd go with option 3

run: pip install -r ${{ github.action_path }}/requirements.txt
run: |
pip install -r requirements.txt # project's dependencies
pip install -r ${{ github.action_path }}/requirements.txt # linters

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

setup-python'scache argument was expecting a requirements.txt but is not doing the install

See the example in their readme file:
https://github.com/actions/setup-python#caching-packages-dependencies

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

yeah, i have noticed this as well. Thanks for the fix

@nmenardg-keepernmenardg-keeper deleted the requirements-not-found-error branchJune 28, 2022 21:22
@nmenardg-keepernmenardg-keeper restored the requirements-not-found-error branchJune 29, 2022 15:34
@nmenardg-keeper
Copy link
Author

Closed by mistake

with:
python-version: "3.8"
- name: Generate requirements.txt file
run: |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm thinking if shouldn't just do this automatically? Check if the requirements file exists (provided by user). If not, install pipenv and generate dependencies.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

If you have time to do that then go ahead, but I don't

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@marian-codemarian-codemarian-code left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@nmenardg-keeper@marian-code

[8]ページ先頭

©2009-2025 Movatter.jp