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.

Sign up for GitHub

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

ci: Add GitHub workflow to run code linting checks daily and on every push or pull request. #4

Merged
merged 6 commits into from
Oct 23, 2024
Merged

ci: Add GitHub workflow to run code linting checks daily and on every push or pull request. #4

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5c16686
ci: Add GitHub workflow to run lint checks daily and on every push or…
kirkrodrigues Oct 23, 2024
73f5f7e
Use pull_request's default settings.
kirkrodrigues Oct 23, 2024
c73b17c
Add lint violation.
kirkrodrigues Oct 23, 2024
3a65e48
Revert "Add lint violation."
kirkrodrigues Oct 23, 2024
7dd08bb
Edit comment.
kirkrodrigues Oct 23, 2024
083d17f
Rename workflow.
kirkrodrigues Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .github/workflows/code-linting-checks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
name: "code-linting-checks"

on:
pull_request:
push:
schedule:
# Run daily at 00:15 UTC (the 15 is to avoid periods of high load)
- cron: "15 0 * * *"
workflow_dispatch:

permissions: {}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding minimum required permissions.

The empty permissions block follows the principle of least privilege, but the workflow might need specific permissions to post lint check results as PR comments.

Consider adding these minimum required permissions:

-permissions: {}
+permissions:
+  pull-requests: write  # To post lint results as PR comments
+  contents: read       # To checkout code
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
permissions: {}
permissions:
pull-requests: write # To post lint results as PR comments
contents: read # To checkout code


concurrency:
group: "${{github.workflow}}-${{github.ref}}"

# Cancel in-progress jobs for efficiency
cancel-in-progress: true

jobs:
lint:
strategy:
matrix:
os: ["macos-latest", "ubuntu-latest"]
runs-on: "${{matrix.os}}"
Comment on lines +20 to +24
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding Windows to the matrix.

The matrix strategy effectively covers macOS and Ubuntu, but consider adding Windows to ensure lint checks pass across all major platforms.

 matrix:
-  os: ["macos-latest", "ubuntu-latest"]
+  os: ["macos-latest", "ubuntu-latest", "windows-latest"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
lint:
strategy:
matrix:
os: ["macos-latest", "ubuntu-latest"]
runs-on: "${{matrix.os}}"
lint:
strategy:
matrix:
os: ["macos-latest", "ubuntu-latest", "windows-latest"]
runs-on: "${{matrix.os}}"

steps:
- uses: "actions/checkout@v4"
with:
submodules: "recursive"

- uses: "actions/setup-python@v5"
with:
python-version: "3.8"

Comment on lines +30 to +33
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using a more recent Python version.

Python 3.8 is approaching end-of-life. Consider using Python 3.11 or 3.12 for better performance and newer features.

 with:
-  python-version: "3.8"
+  python-version: "3.12"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- uses: "actions/setup-python@v5"
with:
python-version: "3.8"
- uses: "actions/setup-python@v5"
with:
python-version: "3.12"

- name: "Install task"
run: "npm install -g @go-task/cli"

Comment on lines +34 to +36
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve task CLI installation.

The global npm installation might require elevated privileges and lacks version pinning.

Consider using a specific version and installing locally:

-run: "npm install -g @go-task/cli"
+run: "npm install @go-task/[email protected]"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: "Install task"
run: "npm install -g @go-task/cli"
- name: "Install task"
run: "npm install @go-task/[email protected]"

- if: "matrix.os == 'macos-latest'"
name: "Install coreutils (for md5sum)"
run: "brew install coreutils"

- name: "Log tool versions"
run: |-
md5sum --version
python --version
tar --version
task --version

- run: "task lint:check"
Loading