-
Notifications
You must be signed in to change notification settings - Fork 20
Add pre-commit config & upgrade devcontainer #411
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
base: main
Are you sure you want to change the base?
Changes from all commits
d903cbb
ff7b3e5
bc2c518
f5b2d0b
be70a50
90239ff
41366a9
ca02bf4
18da09e
7835161
3a887fb
8d82ef3
789fb50
a457665
dd60930
f8aefc6
2e85f47
ddc06d6
5d19316
0d0430c
5334540
f493aac
1d8b12f
53bde54
af5e576
df3f5e0
f86077a
b7588c5
5a0d62c
d3275c8
406037e
019119b
2bd0529
7df25af
206dce7
50256db
19e5af6
c9e9822
eea022c
3444b27
ced2430
92f9c8b
b64929d
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 |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| { | ||
| "name": "eclipse-s-core", | ||
| "image": "ghcr.io/eclipse-score/devcontainer:1.0.0", | ||
| "image": "ghcr.io/eclipse-score/devcontainer:v1.2.0", | ||
| "updateContentCommand": "bazel run //:ide_support" | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,26 +31,29 @@ jobs: | |
| - name: Checkout PR | ||
| uses: actions/checkout@v4.2.2 | ||
|
|
||
| - name: Prepare Python | ||
| run: | | ||
| bazel run //:ide_support | ||
|
|
||
| - name: Prepare report directory | ||
| run: | | ||
| mkdir -p reports | ||
| - name: Build and run dev container task | ||
| uses: devcontainers/ci@v0.3 | ||
| with: | ||
| cacheFrom: ghcr.io/eclipse-score/devcontainer@v1.2.0 | ||
| push: never | ||
| # The pipefail ensures that non 0 exit codes inside the pytest execution get carried into the pipe | ||
| # & make the tests red in the end. Without this we only would check the exit code of the 'tee' command. | ||
| runCmd: | | ||
| set -euxo pipefail | ||
| uv venv && uv pip install -r src/requirements.txt | ||
|
|
||
| # The pipefail ensures that non 0 exit codes inside the pytest execution get carried into the pipe | ||
| # & make the tests red in the end. Without this we only would check the exit code of the 'tee' command. | ||
| - name: Run Consumer tests | ||
| mkdir -p reports | ||
|
|
||
| run: | | ||
| set -o pipefail | ||
| .venv_docs/bin/python -m pytest -s -v src/tests/ --repo="$CONSUMER" --junitxml="reports/${{ matrix.consumer }}.xml" | tee "reports/${{ matrix.consumer }}.log" | ||
| env: | ||
| FORCE_COLOR: "1" | ||
| TERM: xterm-256color | ||
| PYTHONUNBUFFERED: "1" | ||
| CONSUMER: ${{ matrix.consumer }} | ||
| export FORCE_COLOR="1" | ||
| export TERM="xterm-256color" | ||
| export PYTHONUNBUFFERED="1" | ||
| export CONSUMER="${{ matrix.consumer }}" | ||
| export PYTHONPATH=. | ||
| uv run pytest -s -v src/tests/ \ | ||
|
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. pytest is preinstalled in the devcontainer and should be accessible without uv. Maybe uv just runs what is already there
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. Yes pytest is there, but the packages this test needs arent'. So i installe eveyrthing via uv then running it |
||
| --repo="$CONSUMER" \ | ||
| --junitxml="reports/${{ matrix.consumer }}.xml" \ | ||
| | tee "reports/${{ matrix.consumer }}.log" | ||
|
|
||
| - name: Upload consumer test report | ||
| if: ${{ always() }} | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -27,15 +27,9 @@ jobs: | |||||
| - name: Checkout repository | ||||||
| uses: actions/checkout@v4.2.2 | ||||||
|
|
||||||
| - name: Setup Bazel | ||||||
| uses: bazel-contrib/setup-bazel@0.15.0 | ||||||
| - name: Build and run dev container task | ||||||
| uses: devcontainers/ci@v0.3 | ||||||
| with: | ||||||
| disk-cache: true | ||||||
| repository-cache: true | ||||||
| bazelisk-cache: true | ||||||
|
|
||||||
| - name: Install pre-commit | ||||||
| run: pip install pre-commit | ||||||
|
|
||||||
| - name: Run pre-commit checks | ||||||
| run: pre-commit run -a | ||||||
| cacheFrom: ghcr.io/eclipse-score/devcontainer | ||||||
| push: never | ||||||
| runCmd: ${PIPX_BIN_DIR}/pre-commit run -a | ||||||
|
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. With the current
Suggested change
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,33 +22,17 @@ jobs: | |
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4.2.2 | ||
| - name: Cache Bazel and pip | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| ~/.cache/bazel | ||
| ~/.cache/pip | ||
| key: ${{ runner.os }}-test-${{ hashFiles('**/*.bazel', '**/BUILD', '**/*.bzl', 'src/requirements.txt', 'src/**/*.py') }} | ||
|
|
||
| - name: Setup Bazel with cache | ||
| uses: bazel-contrib/setup-bazel@0.15.0 | ||
|
Comment on lines
-33
to
-34
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. We should keep an eye on build times and if we can use a cache with the devcontainer |
||
| - name: Build and run dev container task | ||
| uses: devcontainers/ci@v0.3 | ||
| with: | ||
| disk-cache: true | ||
| repository-cache: true | ||
| bazelisk-cache: true | ||
| - name: Run test targets | ||
| run: | | ||
| bazel run //:ide_support | ||
| bazel test //src/... | ||
| - name: Prepare bundled consumer report | ||
| if: always() | ||
| # Creating tests-report directory | ||
| # Follow Symlinks via '-L' to copy correctly | ||
| # Copy everything inside the 'test-reports' folder | ||
| run: | | ||
| mkdir -p tests-report | ||
| rsync -amL --include='*/' --include='test.xml' --include='test.log' --exclude='*' bazel-testlogs/ tests-report/ | ||
| cacheFrom: ghcr.io/eclipse-score/devcontainer@v1.2.0 | ||
| push: never | ||
| runCmd: | | ||
| bazel run //:ide_support | ||
| bazel test //src/... | ||
| mkdir -p tests-report | ||
| rsync -amL --include='*/' --include='test.xml' --include='test.log' --exclude='*' bazel-testlogs/ tests-report/ | ||
| - name: Upload bundled consumer report | ||
| if: always() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,58 @@ | ||
| # This file is at the root level, as it applies to all Python code, | ||
| # not only to docs or to tools. | ||
| [tool.pyright] | ||
| extends = "bazel-bin/ide_support.runfiles/score_tooling+/python_basics/pyproject.toml" | ||
| typeCheckingMode = "standard" | ||
| pythonVersion = "3.12" # Keep in sync with MODULE.bazel | ||
|
|
||
| # Warn if function parameters lack type annotations | ||
| reportMissingParameterType = "warning" | ||
|
|
||
| # Warn if generic types (e.g. List) are missing type args | ||
| reportMissingTypeArgument = "warning" | ||
|
|
||
| # Warn when using members marked as private (e.g. _log) | ||
| reportPrivateUsage = "warning" | ||
|
|
||
| # Warn when variable type can't be inferred or is 'Any' | ||
| reportUnknownVariableType = "warning" | ||
|
|
||
| # Warn about declared but unused variables | ||
| reportUnusedVariable = "warning" | ||
|
|
||
| exclude = [ | ||
| "**/__pycache__", | ||
| "**/.*", | ||
| "**/bazel-*", | ||
| ".venv*/**", | ||
| ] | ||
|
|
||
| venvPath = "." | ||
| venv = ".venv_docs" | ||
|
|
||
| [tool.ruff] | ||
| extend = "bazel-bin/ide_support.runfiles/score_tooling+/python_basics/pyproject.toml" | ||
|
|
||
| #line-length=59 | ||
| target-version = "py312" # Keep in sync with MODULE.bazel | ||
| extend-exclude = [ | ||
| "__pycache__", | ||
| ".*", | ||
| "bazel-*", | ||
| "**/__pycache__", | ||
| "/.*", | ||
| "bazel-*", | ||
| ".venv*/**", | ||
| ] | ||
|
|
||
| # Selected rules for clean code, readability, and bug prevention | ||
| lint.select = [ | ||
| "E", # pycodestyle (PEP8) | ||
| "F", # pyflakes (undefined vars, unused imports) | ||
| "I", # isort (import sorting) | ||
| "B", # flake8-bugbear (likely bugs) | ||
| "C90", # mccabe (complexity checks) | ||
| "UP", # pyupgrade (modern Python 3.12+ features) | ||
| "SIM", # flake8-simplify (simplifies code patterns) | ||
| "RET" # flake8-return (consistent return statements) | ||
| ] | ||
|
|
||
| lint.ignore = [ | ||
| # Rules we want to ignore go in here. | ||
| # Always provide a comment explaining why. | ||
| ] |
This file was deleted.
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.
Can you add this and tell me if you stell need
PIPX_BIN_DIRto execute pre-commit?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 only works in conjunction with
devcontainer/cli. If this works I can add this to the image.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.
Now I found a solution, which works in
/etc/profile.d/. This took me way too long timeThere 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.
Will be taken care of with eclipse-score/devcontainer#98