Skip to content

feat: updated readme for deploy#40

Merged
edinstance merged 1 commit intodevfrom
ci-fixes
Oct 20, 2025
Merged

feat: updated readme for deploy#40
edinstance merged 1 commit intodevfrom
ci-fixes

Conversation

@edinstance
Copy link
Owner

@edinstance edinstance commented Oct 20, 2025

Summary by CodeRabbit

  • Chores
    • Enhanced CI/CD pipeline reliability by installing required system dependencies across multiple deployment workflows.
    • Updated Python dependency installation methodology to improve consistency across automated deployment processes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 20, 2025

Walkthrough

Six GitHub Actions workflows are updated to add system dependency installation steps. These steps execute apt-update and install libcairo2-dev and pkg-config before proceeding with pip and AWS SAM CLI installation. Additionally, build-and-deploy.yml changes aws-sam-cli installation to use python -m pip syntax.

Changes

Cohort / File(s) Summary
System dependencies installation across workflows
.github/workflows/destroy-infrastructure.yml, .github/workflows/lint-and-format.yml, .github/workflows/rollback.yml, .github/workflows/test-lambdas.yml, .github/workflows/validate-and-lint-sam.yml
Adds new "Install system dependencies" step to run apt-update and install libcairo2-dev and pkg-config before pip upgrade and Python dependency installation.
build-and-deploy.yml with dual changes
.github/workflows/build-and-deploy.yml
Adds "Install system dependencies" step (libcairo2-dev and pkg-config via apt) and changes aws-sam-cli installation from pip install to python -m pip install.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

The changes follow a consistent, repetitive pattern across multiple workflow files. Most modifications involve inserting identical system dependency installation steps. The dual changes in build-and-deploy.yml (system dependencies + pip invocation style change) remain straightforward. Homogeneous nature minimises review complexity despite file count.

Possibly related PRs

  • Created rollback action #25: Modifies .github/workflows/rollback.yml with identical "Install system dependencies" step and aws-sam-cli installation adjustments, representing parallel or prior work on the same workflow.
  • feat: created github actions for this project #6: Introduces the original GitHub Actions workflows now being enhanced with apt-based libcairo2-dev and pkg-config installation steps—foundational work being augmented.

Poem

🐰 System libraries, now in place,
Cairo's ready for the race,
Python's pip in proper dress,
Workflows run without distress! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The PR title states "feat: updated readme for deploy", suggesting that the primary change involves updating documentation or README files. However, the raw summary reveals that the actual changeset consists entirely of modifications to six GitHub Actions workflow files (.github/workflows/*.yml), with no README file being modified at all. The consistent theme across all changes is the addition of a new "Install system dependencies" step to install libcairo2-dev and pkg-config in various CI workflows. This represents CI/CD infrastructure improvements rather than documentation updates, making the title fundamentally misleading and unrelated to the actual work performed. The PR title should be revised to accurately reflect the actual changes, such as "ci: add system dependencies to GitHub Actions workflows" or "ci: install libcairo2-dev and pkg-config across workflows". This would help reviewers and future maintainers understand that the changeset addresses CI/CD workflow requirements rather than documentation updates.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ci-fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
.github/workflows/rollback.yml (1)

47-56: Normalise pip invocation to match build-and-deploy.yml.

Line 56 uses bare pip install aws-sam-cli, but build-and-deploy.yml line 41 correctly uses python -m pip install aws-sam-cli. For consistency and robustness, update line 56 to use python -m pip:

  - name: Upgrade pip and install AWS SAM CLI
    run: |
      python -m pip install --upgrade pip
-     pip install aws-sam-cli
+     python -m pip install aws-sam-cli
.github/workflows/validate-and-lint-sam.yml (1)

21-30: Normalise pip invocation to match build-and-deploy.yml.

Line 30 uses bare pip install aws-sam-cli cfn-lint, but build-and-deploy.yml line 41 correctly uses python -m pip install aws-sam-cli. For consistency and robustness, update line 30 to use python -m pip:

  - name: Upgrade pip and install dependencies
    run: |
      python -m pip install --upgrade pip
-     pip install aws-sam-cli cfn-lint
+     python -m pip install aws-sam-cli cfn-lint
.github/workflows/destroy-infrastructure.yml (1)

41-50: Normalise pip invocation and address the TODO comment for version pinning.

Line 50 uses bare pip install aws-sam-cli, but build-and-deploy.yml line 41 correctly uses python -m pip install aws-sam-cli. For consistency and robustness, normalise to python -m pip. Additionally, the inline TODO comment about pinning the SAM CLI version warrants action—unpinned dependencies can lead to non-deterministic builds and failures if newer versions introduce breaking changes.

  - name: Upgrade pip and install AWS SAM CLI
    run: |
      python -m pip install --upgrade pip
-     pip install aws-sam-cli # Consider pinning SAM CLI version
+     python -m pip install aws-sam-cli==<pinned-version>

Would you like me to identify the currently pinned SAM CLI version used elsewhere in the project and propose a specific version to use?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b78909 and 19f582d.

📒 Files selected for processing (6)
  • .github/workflows/build-and-deploy.yml (1 hunks)
  • .github/workflows/destroy-infrastructure.yml (1 hunks)
  • .github/workflows/lint-and-format.yml (1 hunks)
  • .github/workflows/rollback.yml (1 hunks)
  • .github/workflows/test-lambdas.yml (1 hunks)
  • .github/workflows/validate-and-lint-sam.yml (1 hunks)
🔇 Additional comments (2)
.github/workflows/lint-and-format.yml (1)

20-25: System dependencies step is correctly positioned.

The "Install system dependencies" step is appropriately placed before dependency installation. No blocking issues identified for this workflow.

.github/workflows/test-lambdas.yml (1)

20-25: System dependencies step is correctly positioned.

The "Install system dependencies" step is appropriately placed before dependency installation. No blocking issues identified for this workflow.

Comment on lines +32 to +41
- name: Install system dependencies
run: |
sudo apt update
sudo apt install -y \
libcairo2-dev pkg-config

- name: Upgrade pip and install dependencies
run: |
python -m pip install --upgrade pip
pip install aws-sam-cli
python -m pip install aws-sam-cli
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

System dependencies installation looks sound; ensure consistency across all workflows.

Line 41 correctly uses python -m pip install aws-sam-cli, which is the recommended approach per PEP 394. However, verify that other workflows using aws-sam-cli (rollback.yml, destroy-infrastructure.yml, validate-and-lint-sam.yml) apply the same pip normalisation for consistency and robustness.

Run the following script to check for inconsistent pip invocations across workflows:


🏁 Script executed:

#!/bin/bash
# Check for inconsistent pip invocations in workflow files
rg -n 'pip install' .github/workflows/*.yml | grep -v 'python -m pip'

Length of output: 274


Inconsistent pip invocations found; update three workflows to match build-and-deploy.yml standard.

The verification confirms the concern: three workflows use bare pip install instead of the PEP 394-recommended python -m pip install. Update the following:

  • .github/workflows/validate-and-lint-sam.yml:30 — change pip install aws-sam-cli cfn-lint to python -m pip install aws-sam-cli cfn-lint
  • .github/workflows/rollback.yml:56 — change pip install aws-sam-cli to python -m pip install aws-sam-cli
  • .github/workflows/destroy-infrastructure.yml:50 — change pip install aws-sam-cli to python -m pip install aws-sam-cli
🤖 Prompt for AI Agents
.github/workflows/validate-and-lint-sam.yml around line 30,
.github/workflows/rollback.yml around line 56, and
.github/workflows/destroy-infrastructure.yml around line 50: replace bare "pip
install ..." invocations with the PEP 394-recommended form by changing each
occurrence to "python -m pip install ..." (e.g., change `pip install aws-sam-cli
cfn-lint` to `python -m pip install aws-sam-cli cfn-lint` and the `pip install
aws-sam-cli` entries to `python -m pip install aws-sam-cli`) so all workflows
consistently use the module invocation; keep the rest of the command arguments
identical.

@edinstance edinstance merged commit 59a2506 into dev Oct 20, 2025
4 checks passed
@edinstance edinstance deleted the ci-fixes branch October 20, 2025 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant