Skip to content

chore: test update logdrain README (DO NOT MERGE. DELETE AFTER ENG-2151)#4245

Closed
mcstepp wants to merge 4 commits intomainfrom
test/logdrain-readme-change
Closed

chore: test update logdrain README (DO NOT MERGE. DELETE AFTER ENG-2151)#4245
mcstepp wants to merge 4 commits intomainfrom
test/logdrain-readme-change

Conversation

@mcstepp
Copy link
Collaborator

@mcstepp mcstepp commented Nov 5, 2025

What does this PR do?

Fixes # (issue)

If there is not an issue for this, please create one first. This is used to tracking purposes and also helps us understand why this PR exists

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Ran make fmt on /go directory
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

@linear
Copy link

linear bot commented Nov 5, 2025

@changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2025

⚠️ No Changeset found

Latest commit: 964776b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Nov 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
dashboard Ignored Ignored Preview Nov 5, 2025 9:19pm
engineering Ignored Ignored Preview Nov 5, 2025 9:19pm

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

📝 Walkthrough

Walkthrough

Adds a new GitHub Actions workflow to test the Logdrain application, updates path filtering in the detect_changes workflow to exclude Markdown and text files, and makes minor documentation additions to the Logdrain README.

Changes

Cohort / File(s) Summary
GitHub Actions Workflows
.github/workflows/job_detect_changes.yaml
Modified path filters for Logdrain to exclude *.md and *.txt files from triggering change detection, allowing the detect_changes job to skip non-code changes
GitHub Actions Workflows
.github/workflows/job_test_logdrain.yaml
New workflow file that defines a "Test Logdrain" job triggered via workflow_call, runs TypeScript type checks and a Wrangler dry-run deploy on a self-hosted Ubuntu 24.04 runner
GitHub Actions Workflows
.github/workflows/pr.yaml
Added new "Test Logdrain" job that runs conditionally when logdrain changes are detected, mirrors the existing Dashboard test job structure
Documentation
apps/docs/docs.json
Reformatted two single-property arrays from multi-line to inline format in Concepts/Identities and Errors/User Errors sections without semantic changes
Documentation
apps/logdrain/README.md
Added Overview section describing Logdrain as a Cloudflare Worker that receives and processes logs

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Workflow changes follow established GitHub Actions patterns with minimal logic density
  • Path filter modification is a straightforward glob pattern addition
  • New test workflow mirrors existing patterns with standard steps (checkout, setup, type-check, deploy)
  • Documentation updates are additive and non-breaking

Possibly related PRs

Suggested labels

Core Team, Papercut

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is completely unfilled with only placeholder template text, no actual summary of changes, no issue reference, no change type marked, and no test instructions provided. Complete the description by filling in the actual changes made, referencing the issue number (ENG-2151), marking the change type, and describing how to test the workflow and README changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title is partially related to the changeset, referring to a logdrain README update, but misses major changes like GitHub workflow additions and formatting updates to docs.json.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch test/logdrain-readme-change

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a1f538 and 964776b.

📒 Files selected for processing (5)
  • .github/workflows/job_detect_changes.yaml (1 hunks)
  • .github/workflows/job_test_logdrain.yaml (1 hunks)
  • .github/workflows/pr.yaml (1 hunks)
  • apps/docs/docs.json (2 hunks)
  • apps/logdrain/README.md (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3755
File: .github/workflows/runbook-freshness-check.yaml:157-173
Timestamp: 2025-08-08T14:59:52.283Z
Learning: Repo unkeyed/unkey: When a CI/workflow fix is deferred, imeyer prefers a thorough GitHub issue be opened with sections (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References) and assigned to imeyer, including backlinks to the originating PR and comment.
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3755
File: .github/actions/setup-node/action.yaml:0-0
Timestamp: 2025-08-08T15:10:46.436Z
Learning: Repo: unkeyed/unkey — Preference: If imeyer comments “issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3755
File: .github/workflows/job_detect_changes.yaml:111-114
Timestamp: 2025-08-08T16:10:00.224Z
Learning: Repo: unkeyed/unkey — When imeyer writes “make me an issue” on a PR thread, automatically open a thorough GitHub issue (Summary, Impact, Where, Repro, Observed vs Expected, Acceptance Criteria, Validation Plan, Out of Scope, References), include backlinks to the PR and comment, and assign to imeyer.
📚 Learning: 2025-08-08T19:52:42.113Z
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3765
File: .github/workflows/job_detect_changes.yaml:43-43
Timestamp: 2025-08-08T19:52:42.113Z
Learning: In the unkeyed/unkey repository, the workflows `.github/workflows/job_test_api_local.yaml` and `.github/workflows/job_test_go_api_local.yaml` should keep their Blacksmith self-hosted runners (blacksmith-4vcpu-ubuntu-2404 and blacksmith-8vcpu-ubuntu-2404 respectively) as these are performance-critical test jobs that require dedicated resources.

Applied to files:

  • .github/workflows/pr.yaml
  • .github/workflows/job_test_logdrain.yaml
📚 Learning: 2024-10-15T19:57:16.520Z
Learnt from: Devansh-Baghel
Repo: unkeyed/unkey PR: 2452
File: oss.gg/7_create_a_template.md:42-42
Timestamp: 2024-10-15T19:57:16.520Z
Learning: In Hono & Cloudflare Workers templates, the 'Setup and Installation' section in the README serves as the quickstart guide.

Applied to files:

  • apps/logdrain/README.md
🪛 actionlint (1.7.8)
.github/workflows/job_test_logdrain.yaml

9-9: label "depot-ubuntu-24.04-4" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-11-arm", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-26-xlarge", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file

(runner-label)

🔇 Additional comments (6)
apps/logdrain/README.md (1)

3-4: LGTM.

Clear and concise project overview.

.github/workflows/job_detect_changes.yaml (1)

103-104: Path filtering excludes documentation changes.

The updated patterns will prevent README and .txt file changes from triggering the logdrain test workflow. This is likely intentional for this test PR, but note that the README.md update in this PR won't trigger CI when evaluated against these filters.

.github/workflows/job_test_logdrain.yaml (2)

20-27: Workflow structure is sound.

The type check and dry-run deploy steps are appropriate validation for a Cloudflare Worker. Setup actions and environment variable (CI=1) follow established patterns in the codebase.


9-9: Ignore this review comment—actionlint is not configured in this repository.

No actionlint setup exists in the codebase (no configuration file, no CI invocation, no dependencies). The depot-ubuntu-24.04-4 runner is legitimately used across 25+ workflows without issues. The suggestion to register it in .github/actionlint.yaml is invalid since neither actionlint nor that configuration file exist. No changes required.

Likely an incorrect or invalid review comment.

.github/workflows/pr.yaml (1)

43-47: Integration is clean.

The test_logdrain job follows established patterns, properly gates execution on detect_changes.outputs.logdrain == 'true', and correctly references the new test workflow template. Conditional logic matches other test jobs in the file.

apps/docs/docs.json (1)

116-116: Formatting changes only.

Array elements are unchanged; only whitespace formatting was adjusted. No impact on navigation or documentation structure.

Also applies to: 272-272


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.

@mcstepp mcstepp mentioned this pull request Nov 5, 2025
19 tasks
@mcstepp mcstepp force-pushed the test/logdrain-readme-change branch from 1356e0a to 16e6bb0 Compare November 5, 2025 21:18
@vercel vercel bot temporarily deployed to Preview – dashboard November 5, 2025 21:19 Inactive
@mcstepp mcstepp closed this Nov 6, 2025
@mcstepp mcstepp deleted the test/logdrain-readme-change branch November 6, 2025 16:01
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