-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fix devcontainer #516
Fix devcontainer #516
Conversation
Reviewer's Guide by SourceryThis pull request focuses on fixing the development container setup. It includes updates to pre-commit configurations and the Dockerfile to ensure a smoother development experience, particularly for building and testing the pyatmo library. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @pectum83 - I've reviewed your changes - here's some feedback:
Overall Comments:
- It looks like you've made changes to the pre-commit configuration; please ensure these changes don't introduce new linting errors.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
.pre-commit-config.yaml
Outdated
exclude: ^(fixtures/) | ||
|
||
repos: | ||
- repo: https://github.com/pycqa/flake8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pyatmo has switched to ruff
for style guide enforcement, so there is no need to reintroduce flake8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. I supressed it also in postCreateCommand of devcontainer
.devcontainer/Dockerfile.dev
Outdated
|
||
FROM mcr.microsoft.com/vscode/devcontainers/python:0-${VARIANT} as builder | ||
|
||
SHELL ["/bin/bash", "-o", "pipefail", "-c"] | ||
|
||
WORKDIR /workspaces | ||
|
||
COPY Pipfile ./ | ||
COPY Pipfile* ./ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line could be remove as pyatmo no longer use pipenv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I also suppress all pipenv related lines
I did the changes. I am a beginner with github. Do I need to do something on the PR after I did new commits ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi, I plan to add NAC netatmo clim in pyatmo in order to use it in home assistant.
This first PR is just about what I had to modify in order to build the devcontainer and have pytest pass on current dev branch. I am not sure it will work for any configuration but now it works for mine.
Summary by Sourcery
Update the development container configuration to resolve build issues and ensure tests pass. Update pre-commit hooks.
Build: