Use Deno as a upload-git-commit-notion language #947
Merged
Conversation
github-actions
Bot
added
🎉 New Feature
There was a problem hiding this comment.
Pull request overview
This PR migrates the upload-git-commit-notion pre-push hook from a Bash script to a type-safe Deno/TypeScript implementation, and introduces CI to lint/format/type-check Deno projects in the repo.
Changes:
- Replace the Bash-based Notion uploader hook with a Deno/TypeScript hook (plus
deno.json/deno.lock). - Update hook metadata to run the new Deno entrypoint.
- Add a GitHub Actions workflow intended to lint/format/type-check Deno projects.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents that prek is required to run the hooks. |
| pre-commit/scripts/upload-git-commit-notion | Removes the previous Bash implementation of the hook. |
| pre-commit/hooks/upload-git-commit-notion/main.ts | Adds the new Deno/TypeScript hook implementation. |
| pre-commit/hooks/upload-git-commit-notion/deno.json | Defines Deno imports and permissions for the hook. |
| pre-commit/hooks/upload-git-commit-notion/deno.lock | Locks Deno/JSR/NPM dependencies used by the hook. |
| .pre-commit-hooks.yaml | Switches the hook entry/language to the Deno implementation. |
| .github/workflows/ci.yml | Adds CI checks for Deno projects (currently with some blocking issues). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5ouma
force-pushed
the
feat-pre-commit-upload-git-commit-notion-deno
branch
from
1705dfa to
3eb9a81
Compare
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
WalkthroughThis PR introduces two independent improvements: a GitHub Actions CI workflow for Deno projects that auto-detects and validates TypeScript modules, and a migration of the upload-git-commit-notion pre-commit hook from bash to Deno TypeScript with proper dependency pinning and Notion API integration. ChangesPre-commit Hook Migration to Deno
GitHub Actions CI Workflow
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 4
🧹 Nitpick comments (2)
pre-commit/hooks/upload-git-commit-notion/main.ts (2)
22-28: ⚡ Quick winImprove type safety and error handling for 1Password integration.
The code checks if
datais an array (line 26) but then assumes each element has avalueproperty without validation. If the 1Password item structure changes or fields are missing, this could fail with unclear errors.♻️ Proposed improvement
const [database_id, auth] = (() => { const data = item.get("Notion Git Integration", { fields: { label: ["database-id", "credential"] }, }); - if (!Array.isArray(data)) throw new Error("Data is not an array"); - return data.map((field) => field.value); + if (!Array.isArray(data) || data.length !== 2) { + throw new Error("Expected exactly 2 fields from 1Password item"); + } + const [dbField, authField] = data; + if (!dbField?.value || !authField?.value) { + throw new Error("Missing required field values from 1Password item"); + } + return [dbField.value, authField.value]; })();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pre-commit/hooks/upload-git-commit-notion/main.ts` around lines 22 - 28, The current inline extraction assumes every element returned by item.get has a value property and correct labels; add explicit validation after retrieving data from item.get("Notion Git Integration", ...) to ensure it's an array of objects with the expected label names ("database-id" and "credential") and that each object has a non-empty value, and if not throw a descriptive error; update the extraction (used to set database_id and auth) to locate values by label (or index) with null checks and clear error messages referencing the symbols item.get, data, database_id and auth so failures in 1Password structure are reported immediately and deterministically.
31-47: 🏗️ Heavy liftConsider adding error handling for Notion API calls.
The Notion page creation loop doesn't handle potential API failures (rate limits, network issues, invalid data). If one commit fails to upload, the entire hook will fail and subsequent commits won't be processed.
♻️ Proposed improvement with error handling
for (const commit of commits) { - const res = await client.pages.create({ - parent: { database_id }, - properties: { - Message: { title: [{ type: "text", text: { content: commit.subject } }] }, - ...(commit.body && { - Description: { - rich_text: [{ type: "text", text: { content: commit.body } }], - }, - }), - Hash: { rich_text: [{ type: "text", text: { content: commit.short } }] }, - URL: { url }, - Date: { date: { start: commit.author.date.toInstant().toString() } }, - }, - }); - console.log(res.id); + try { + const res = await client.pages.create({ + parent: { database_id }, + properties: { + Message: { title: [{ type: "text", text: { content: commit.subject } }] }, + ...(commit.body && { + Description: { + rich_text: [{ type: "text", text: { content: commit.body } }], + }, + }), + Hash: { rich_text: [{ type: "text", text: { content: commit.short } }] }, + URL: { url }, + Date: { date: { start: commit.author.date.toInstant().toString() } }, + }, + }); + console.log(res.id); + } catch (error) { + console.error(`Failed to upload commit ${commit.short}: ${error.message}`); + // Continue processing other commits + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pre-commit/hooks/upload-git-commit-notion/main.ts` around lines 31 - 47, Wrap the Notion API call inside the for (const commit of commits) loop (the client.pages.create call) in a try/catch so a single failing commit doesn't abort the whole process; in the catch log the error with identifying data (e.g., commit.short and commit.subject) and continue to the next commit. Additionally, for transient failures/rate limits implement a simple retry-with-backoff around client.pages.create (detect 429/timeouts) before giving up, and surface a non-fatal summary at the end if any commits failed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 78-79: The CI step using the command `deno install --frozen` will
fail if no deno.lock is present; update the workflow or repo so a committed
`deno.lock` exists before this step runs or change the step to a non-frozen
install (remove `--frozen`) until the lock is added; locate the job that
contains the `if: ${{ steps.setup-deno.outputs.cache-hit != 'true' }}` and the
`run: deno install --frozen` command and either ensure `deno.lock` is
committed/checked out prior to this run or replace `deno install --frozen` with
`deno install` temporarily.
- Around line 37-39: The workflow currently only triggers the lockfile check
when deno.lock is "added|modified", so new repos without a committed deno.lock
are never checked; update the CI job that contains the filters -> lockfile block
to either remove the restrictive filters so the job runs on all PRs/pushes or
change the logic to explicitly detect a missing deno.lock (e.g., add a pre-check
step that fails if deno.lock is absent) and ensure the workflow references
deno.lock (the lockfile check) for the upload-git-commit-notion hook so new Deno
projects without a committed deno.lock are caught.
In `@pre-commit/hooks/upload-git-commit-notion/main.ts`:
- Line 3: Remove the debug console.log("hello") statement from
pre-commit/hooks/upload-git-commit-notion/main.ts; locate the
console.log("hello") call and delete it so no debug output is printed during
production runs.
- Around line 10-20: The code builds range.from/to from
Deno.env.get("PRE_COMMIT_FROM_REF") and Deno.env.get("PRE_COMMIT_TO_REF")
without validation before calling git().commit.log(range); add explicit
validation: read each env into local constants (e.g., fromRef, toRef), check
they are non-empty, and if either is missing throw a clear Error naming the
missing PRE_COMMIT_FROM_REF or PRE_COMMIT_TO_REF; then set range = { from:
fromRef, to: toRef } and call git().commit.log(range) as before so the failure
mode and error message are deterministic.
---
Nitpick comments:
In `@pre-commit/hooks/upload-git-commit-notion/main.ts`:
- Around line 22-28: The current inline extraction assumes every element
returned by item.get has a value property and correct labels; add explicit
validation after retrieving data from item.get("Notion Git Integration", ...) to
ensure it's an array of objects with the expected label names ("database-id" and
"credential") and that each object has a non-empty value, and if not throw a
descriptive error; update the extraction (used to set database_id and auth) to
locate values by label (or index) with null checks and clear error messages
referencing the symbols item.get, data, database_id and auth so failures in
1Password structure are reported immediately and deterministically.
- Around line 31-47: Wrap the Notion API call inside the for (const commit of
commits) loop (the client.pages.create call) in a try/catch so a single failing
commit doesn't abort the whole process; in the catch log the error with
identifying data (e.g., commit.short and commit.subject) and continue to the
next commit. Additionally, for transient failures/rate limits implement a simple
retry-with-backoff around client.pages.create (detect 429/timeouts) before
giving up, and surface a non-fatal summary at the end if any commits failed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e4c66416-f2c2-4f82-ac71-70b7c3362a80
⛔ Files ignored due to path filters (1)
pre-commit/hooks/upload-git-commit-notion/deno.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/workflows/ci.yml.pre-commit-hooks.yamlREADME.mdpre-commit/hooks/upload-git-commit-notion/deno.jsonpre-commit/hooks/upload-git-commit-notion/main.tspre-commit/scripts/upload-git-commit-notion
💤 Files with no reviewable changes (1)
- pre-commit/scripts/upload-git-commit-notion
Manage the hook as type-safe.
The original one doesn't support Deno language.
5ouma
force-pushed
the
feat-pre-commit-upload-git-commit-notion-deno
branch
5 times, most recently
from
a4b5885 to
db64664
Compare
Check format and lint the files.
5ouma
force-pushed
the
feat-pre-commit-upload-git-commit-notion-deno
branch
2 times, most recently
from
643ae58 to
1e6fb89
Compare
github-actions
Bot
added
🏷️ Label Past Pull Requests
5ouma
force-pushed
the
feat-pre-commit-upload-git-commit-notion-deno
branch
from
1e6fb89 to
82d90dd
Compare
github-actions
Bot
removed
🏷️ Label Past Pull Requests
5ouma
force-pushed
the
feat-pre-commit-upload-git-commit-notion-deno
branch
from
82d90dd to
a317161
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Manage the hook as type-safe.