Skip to content

Conversation

@Meldiron
Copy link
Contributor

@Meldiron Meldiron commented Oct 23, 2025

What does this PR do?

Adds CI/CD job to warn against large files. Should help prevent uploading large pngs by mistake

Test Plan

  • test must pass

Related PRs and Issues

x

Have you read the Contributing Guidelines on issues?

yes

Summary by CodeRabbit

  • Chores
    • Added an automated "Large file" warning for pull requests that detects common image formats and excludes build and dependency folders.
    • Automatically runs image compression with sensible quality presets when meaningful size reductions are possible, helping keep PRs and the repository lean.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

A new GitHub Actions workflow file is added at .github/workflows/lfs-warning.yml named "Large file warning". It triggers on pull_request for all branches and filters image file paths (jpg, jpeg, png, webp, avif). The job lfs-warning runs on ubuntu-latest, checks out the PR head SHA using actions/checkout@v4 (ref set to PR head SHA), and runs a "Compress Images" step using calibreapp/image-actions@main with inputs for JPEG/PNG/WebP/AVIF quality, ignored paths (node_modules/**,build), minPctChange, and compressOnly. A GITHUB_TOKEN line is present but commented out.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The PR title "Feat: LFS warning" uses "LFS," which in development contexts typically refers to Git Large File Storage, a specific git extension tool. However, the actual implementation introduces a GitHub Actions workflow using the calibreapp/image-actions tool to compress images rather than implementing Git LFS or a generic warning system. While the PR objective mentions warning against large files, the actual changeset performs automatic image compression, not just warning. This creates a disconnect between the title's terminology and what the PR actually implements, potentially misleading developers scanning the git history about the nature of this change. Consider revising the title to more accurately reflect the actual implementation, such as "Feat: Add image compression workflow" or "Feat: Automate image compression on pull requests" to clarify that this is about automated image compression rather than LFS or a simple warning system. This would better align the title with what developers will find when reviewing the actual code changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-lfs-warning

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.

❤️ Share

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: 0

🧹 Nitpick comments (2)
.github/workflows/lfs-warning.yml (2)

8-18: Rename job from "format" to be more descriptive.

The job is named format, which conventionally refers to code formatting workflows (e.g., Prettier, Black). However, this job performs file size checking for LFS warnings. A more descriptive name like large-file-check or lfs-warning-check would improve clarity and prevent confusion when viewing CI status or referencing the job in other workflows.

Apply this diff to improve clarity:

 jobs:
-    format:
+    large-file-check:
         runs-on: ubuntu-latest

1-18: Consider updating to ppremk/lfs-warning@v3.3 (latest).

ppremk/lfs-warning@v3.2 is a legitimate published release, but v3.3 is available. While v3.2 is functional and valid, updating to the latest version ensures access to any recent improvements or fixes.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f34c326 and 4bf3aff.

📒 Files selected for processing (1)
  • .github/workflows/lfs-warning.yml (1 hunks)
⏰ 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). (2)
  • GitHub Check: tests
  • GitHub Check: build

@github-actions
Copy link

Images automagically compressed by Calibre's image-actions

Compression reduced images by 73.1%, saving 365.7 KB.

Filename Before After Improvement Visual comparison
static/sample-delme.png 500.6 KB 134.9 KB 73.1% View diff

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: 0

🧹 Nitpick comments (1)
.github/workflows/lfs-warning.yml (1)

21-21: Pin the action to a stable version tag, not @main.

Using @main for CI/CD workflows can introduce unexpected breaking changes. Specify a concrete version tag (e.g., @v1.1.0) to ensure reproducible builds and prevent surprise failures from upstream changes.

-      uses: calibreapp/image-actions@main
+      uses: calibreapp/image-actions@v1.1.0

Verify the latest stable version on the action's releases page.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4bf3aff and 2872f8a.

⛔ Files ignored due to path filters (1)
  • static/sample-delme.png is excluded by !**/*.png
📒 Files selected for processing (1)
  • .github/workflows/lfs-warning.yml (1 hunks)
🔇 Additional comments (1)
.github/workflows/lfs-warning.yml (1)

23-31: Clarify the workflow's purpose: warning vs. automatic compression.

The PR objective states "warn against large files," but this workflow is configured to compress and potentially commit changes (compressOnly: false). Additionally, the GITHUB_TOKEN is commented out, which may prevent the action from functioning as intended (e.g., posting comments or creating commits).

Consider one of these approaches:

  1. If the goal is to warn only (not modify files): Set compressOnly: true and uncomment GITHUB_TOKEN so the action can comment on the PR with warnings.
  2. If the goal is to automatically compress (modify files in the PR): Uncomment GITHUB_TOKEN so the action has permission to commit changes.

Which approach is intended?

@Meldiron
Copy link
Contributor Author

Dont like any of directions. Ill re-understand original problem, and see how else can it be prevented

@Meldiron Meldiron closed this Oct 23, 2025
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.

2 participants