Skip to content

Addressing PR comments#80

Merged
don-petry merged 4 commits intomainfrom
add-claude-md
Mar 27, 2026
Merged

Addressing PR comments#80
don-petry merged 4 commits intomainfrom
add-claude-md

Conversation

@don-petry
Copy link
Copy Markdown
Collaborator

@don-petry don-petry commented Mar 27, 2026

Summary by CodeRabbit

  • Documentation

    • Added a repository guide covering setup, installing dependencies, running tests with coverage, lint/format commands, and required pre-commit steps; clarifies mandatory development rules (TDD-first, coverage thresholds, testing expectations) and describes a recommended Apps Script testing pattern and project structure, with a pointer to broader agent guidance.
  • Note

    • No user-facing changes.

Copilot AI review requested due to automatic review settings March 27, 2026 03:31
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 991bae80-353d-4b9f-94ac-281622c08504

📥 Commits

Reviewing files that changed from the base of the PR and between 64b0e8e and 3e7a463.

📒 Files selected for processing (1)
  • CLAUDE.md
✅ Files skipped from review due to trivial changes (1)
  • CLAUDE.md

📝 Walkthrough

Walkthrough

Added a new CLAUDE.md documentation file that specifies Claude Code repository operating procedures: install/test/lint commands, mandatory TDD, coverage thresholds and restrictions, Google Apps Script testing pattern, required pre-commit commands, and expected Apps Script project architecture.

Changes

Cohort / File(s) Summary
Documentation
CLAUDE.md
New file introducing Claude Code-specific development rules and workflow: dependency/install commands, test/coverage/lint/format commands, mandatory TDD and coverage requirements, GAS testing pattern (extract logic to src/index.js with module.exports and injectable services), required pre-commit commands, and expected src/ architecture mapping.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Addressing PR comments' is vague and does not clearly describe the main change, which is adding a CLAUDE.md file with Claude Code-specific instructions. Consider a more descriptive title like 'Add CLAUDE.md for Claude Code-specific context' that clearly indicates the primary change.
✅ 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 changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch add-claude-md

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

Copy link
Copy Markdown

@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: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLAUDE.md`:
- Line 20: Update the pre-commit guidance to avoid redundancy and clarify
purpose: replace the line instructing to run "npm run check and npm test --
--coverage" with a clear pair of commands—run "npm run check" (which only runs
prettier and lint) and run "npm test" (the package.json "test": "jest
--coverage" already applies coverage), and remove the extra "-- --coverage"
mention so readers aren't misled about what "npm run check" does.
- Around line 8-9: Remove the redundant coverage command from the quick
reference: delete the `npm test -- --coverage` entry so the file only documents
`npm test` (since `npm test` already runs `jest --coverage` per package.json);
ensure the remaining text mentions that `npm test` includes coverage to avoid
confusion and update any adjacent bullet formatting if needed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e12f1a70-673e-4af9-8eb1-eb995ab92bee

📥 Commits

Reviewing files that changed from the base of the PR and between 88cc890 and 7c629d3.

📒 Files selected for processing (1)
  • CLAUDE.md

Comment thread CLAUDE.md Outdated
Comment thread CLAUDE.md Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a Claude Code–specific context file (CLAUDE.md) to help agents work effectively in this Google Apps Script repo, while deferring to AGENTS.md as the canonical guidance.

Changes:

  • Introduces CLAUDE.md with quick reference commands, key rules, and a brief architecture summary.
  • Links to AGENTS.md for comprehensive repository guidelines.

Comment thread CLAUDE.md Outdated
Comment thread CLAUDE.md Outdated
Comment thread CLAUDE.md Outdated
@don-petry
Copy link
Copy Markdown
Collaborator Author

@claude - review and address comments. Avoid duplicating content from AGENTS.md . Merge when ready.

@don-petry
Copy link
Copy Markdown
Collaborator Author

@claude[agent] - review and address comments. Avoid duplicating content from AGENTS.md . Merge when ready.

@Claude
Copy link
Copy Markdown
Contributor

Claude AI commented Mar 27, 2026

@claude[agent] - review and address comments. Avoid duplicating content from AGENTS.md . Merge when ready.

Addressed all review comments in 64b0e8e:

  • Fixed product name: "Google App Scripts" → "Google Apps Script"
  • Clarified "Lint + Format" → "Lint + format check" (since npm run check only validates, doesn't apply formatting)
  • Aligned coverage-ignore guidance with AGENTS.md to allow rare, justified exceptions

Waiting for CI checks to complete before merging.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh gh pr view --json state,mergeable,statusCheckRollup /home/REDACTED/.nvm/bash_completion /home/REDACTED/.nvm/install.sh /home/REDACTED/.nvm/nvm-exec /home/REDACTED/.nvm/nvm.sh /home/REDACTED/.nvm/package.json /home/REDACTED/.nvm/rename_test.sh /home/REDACTED/.nvm/test -d d -n 10 (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@Claude Claude AI changed the title Add CLAUDE.md for Claude Code Addressing PR comments Mar 27, 2026
npm test already runs jest --coverage per package.json, so the separate
coverage entry was redundant. Also clarified the pre-commit rule to
note what each command does.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

@don-petry don-petry merged commit 0fc3e49 into main Mar 27, 2026
13 checks passed
@don-petry don-petry deleted the add-claude-md branch March 27, 2026 14:42
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.

3 participants