Skip to content
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

Improve devcontainer experience #2964

Merged
merged 1 commit into from
May 5, 2024

Conversation

chrschorn
Copy link
Contributor

@chrschorn chrschorn commented Apr 27, 2024

Starting from a fresh clone, I faced these issues inside a no-cache build of the devcontainer:

isort is missing when you try to run python main.py -> I added it to the poetry dev dependencies

pre-commit run doesn't work out of the box (see screenshot below). This is because the way the devcontainer is setup is a little odd:

  • At build time, the source folder ("nicegui") is copied to the current directory and then installed
    COPY nicegui pyproject.toml poetry.lock README.md ./
    RUN poetry install --all-extras
    With that, you end up with a copy of the nicegui/ folder contents in / which is registered as a Python module (due to the poetry install), leading to a circular import. The source files are not needed to setup the dependencies in the container, which can be achieved with poetry install --all-extras --no-root and just the poetry.lock.
  • The .devcontainer has an extra }, causing this to never run:
    "remoteUser": "vscode",
    "postCreateCommand": "poetry install --all-extras"
    I removed the } and now it's correctly executed. Technically the devcontainer dockerfile wouldn't need to run poetry at all now which is worth considering.

Something else I noticed but didn't want to touch because I don't know how it impacts CI/CD: CONTRIBUTING.md mentions autopep8 --max-line-length=120 --in-place --recursive . It would be nice if this was included in pre-commit to not have to run multiple commands before commit/push.

Screenshot

@falkoschindler falkoschindler added this to the 1.4.24 milestone Apr 29, 2024
@falkoschindler falkoschindler added the enhancement New feature or request label Apr 29, 2024
pyproject.toml Show resolved Hide resolved
Copy link
Member

@rodja rodja left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @chrschorn. I tested the setup and it works as expected.

@rodja rodja merged commit 62e9ec3 into zauberzeug:main May 5, 2024
1 check passed
@rodja
Copy link
Member

rodja commented May 5, 2024

Something else I noticed but didn't want to touch because I don't know how it impacts CI/CD: CONTRIBUTING.md mentions autopep8 --max-line-length=120 --in-place --recursive . It would be nice if this was included in pre-commit to not have to run multiple commands before commit/push.

You are totally right! Would you like to create a pull request for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants