-
-
Notifications
You must be signed in to change notification settings - Fork 113
Add Debian CI which is compatible with docker-pgrouting build #326
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
Conversation
WalkthroughA new GitHub Actions workflow file is added to automate building Debian packages across multiple PostgreSQL (14–18) and PostGIS (3.5–3.6) version combinations using containerized builds with standard installation and compilation steps. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/debian.yml (4)
3-8: Misleading comment and consider adding branch filters.The comment states "manually triggered workflow" but the workflow also triggers on
pushandpull_requestevents. Additionally, without branch filters, this will run on every push to any branch.💡 Suggested improvement
-# manually triggered workflow +# triggered on push/PR to main branches or manually on: workflow_dispatch: push: + branches: + - main + - develop pull_request: + branches: + - main + - develop
23-49: Consider simplifying the matrix definition.The current matrix with extensive excludes effectively results in only 5 combinations:
(18, 3.6),(17, 3.5),(16, 3.5),(15, 3.5),(14, 3.5). An explicitinclude-only approach may be clearer and easier to maintain.💡 Alternative explicit matrix
strategy: fail-fast: false matrix: include: - postgres: 18 postgis: 3.6 pqxx: '7.10' - postgres: 17 postgis: 3.5 pqxx: '6.4' - postgres: 16 postgis: 3.5 pqxx: '6.4' - postgres: 15 postgis: 3.5 pqxx: '6.4' - postgres: 14 postgis: 3.5 pqxx: '6.4'
71-81: Minor: Consider usingworking-directoryand fix step name.The step name "Configure compiler" is slightly misleading—it's configuring the CMake build system. Also, using the
working-directorydirective is cleaner thancd.💡 Suggested improvement
- - name: Configure compiler + - name: Configure build run: | mkdir build cd build cmake .. - name: Build + working-directory: build run: | - cd build make -j $(nproc) make install
57-69: AddDEBIAN_FRONTEND=noninteractivefor defensive CI practices.While the
libpqxxpackage naming (libpqxx-6.4 and libpqxx-7.10) is already correct for Debian, settingDEBIAN_FRONTEND=noninteractiveis a defensive best practice in CI/container environments to explicitly prevent any interactive prompts.💡 Suggested improvement
- name: Install dependencies run: | + export DEBIAN_FRONTEND=noninteractive apt update
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/debian.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
.github/workflows/debian.yml (2)
11-16: LGTM!Good practices: concurrency control prevents redundant parallel runs, and permissions follow the principle of least privilege with read-only access.
51-52: Allpostgis/postgisimage tags used in the matrix are available on Docker Hub, including the PostgreSQL 18 combination (18-3.6). No action required.
Changes proposed in this pull request:
@pgRouting/admins
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.