Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions .github/workflows/codeowner_assignment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:

steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6

- uses: actions/create-github-app-token@29824e69f54612133e76f7eaac726eef6c875baf # v2.2.1
id: token
with:
Expand Down Expand Up @@ -45,9 +45,9 @@ jobs:
echo "Changed files:"
echo "$CHANGED_FILES" | tr ' ' '\n' | sed 's/^/- /'
echo "----------------------------------------"
# Get existing reviewers
# merge reqeusted teams and users into a single array
# merge requested teams and users into a single array
REQUESTED_REVIEWERS=$(gh pr view $PR_NUMBER --json reviewRequests --jq '[.reviewRequests[] | if .__typename == "Team" then .slug else .login end]')
echo "Requested reviewers:"
echo "$REQUESTED_REVIEWERS" | tr ' ' '\n' | sed 's/^/- /'
Expand All @@ -56,7 +56,7 @@ jobs:
# Parse CODEOWNERS and find commented lines
# Add newline to the end of the file if it doesn't have one, otherwise sed will not read the last line
sed -i -e '$a\' $codeowner_path
while read -r LINE; do
# Skip lines that are not commented, GitHub will take care of un-commented lines
if [[ ! "$LINE" =~ ^# ]]; then continue; fi
Expand All @@ -73,7 +73,7 @@ jobs:
# Convert pattern to a regex for matching
REGEX_PATTERN=$(echo "$PATTERN" | sed -e 's/\./\\./g' -e 's/\*/.*/g' -e 's/\?/./g' -e 's|^/||')
# Match changed files to the pattern
for FILE in $CHANGED_FILES; do
# For glob patterns(e.g. "**/"), use a different matching approach
Expand Down Expand Up @@ -134,4 +134,3 @@ jobs:
fi
done
done < $codeowner_path
22 changes: 17 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
default_language_version:
node: latest
node: "22"
exclude: '\.snap|node_modules$'
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
Expand All @@ -23,13 +23,25 @@ repos:
- id: flake8
- id: fix-encoding-pragma
args: ['--remove']
- repo: https://github.com/getsentry/pre-commit-hooks
rev: f3237d2d65af81d435c49dee3593dc8f03d23c2d
- repo: local
hooks:
- id: prettier
entry: node_modules/.bin/prettier
name: Prettier
description: This hook runs Prettier on JavaScript files
entry: prettier
language: node
files: '\.jsx?$'
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
exclude: 'node_modules'
types: [text]
args: ['--write']
- id: eslint
entry: node_modules/.bin/eslint
name: eslint
description: This hook runs eslint on JavaScript files
entry: eslint
language: node
files: '\.jsx?$'
exclude: 'node_modules'
types: [text]
Comment on lines +34 to +44
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The local prettier and eslint hooks use language: node without additional_dependencies, which will cause them to fail with 'executable not found' errors during commits.
Severity: MEDIUM

Suggested Fix

To fix this, either change the language to language: system and point the entry to the project's local node_modules/.bin executables, or add additional_dependencies: [prettier, eslint] to the hook definitions to ensure the tools are installed in the isolated hook environment.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: .pre-commit-config.yaml#L28-L44

Potential issue: The `pre-commit` configuration for the local `prettier` and `eslint`
hooks specifies `language: node`. This creates an isolated, empty Node.js environment
for the hooks. Because `additional_dependencies` are not provided, the `prettier` and
`eslint` packages will not be installed into this environment. As a result, when a
developer attempts to commit a file that triggers these hooks, the `pre-commit`
framework will be unable to find the `prettier` or `eslint` executables, causing the
hook to fail and blocking the commit.

Did we get this right? 👍 / 👎 to inform future reviews.

- repo: https://github.com/crate-ci/typos
rev: v1.39.0
hooks:
Expand Down
3 changes: 2 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ Use **pnpm**: `pnpm install`, `pnpm dev`, `pnpm build`, `pnpm test`

## Code Style

- ESLint + Prettier enforced via pre-commit hooks
- ESLint + Prettier enforced via pre-commit hooks using `prek`
- If you previously installed `pre-commit` and `prek` is available on your system, reinstall the Git shims with `prek install -f`.
- Use TypeScript strict mode
- Follow existing patterns in codebase

Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ develop: setup-git
npx -y @sentry/dotagents install

setup-git:
ifneq (, $(shell which pre-commit))
pre-commit install
ifneq (, $(shell which prek))
prek install
endif
git config branch.autosetuprebase always

Expand Down
10 changes: 5 additions & 5 deletions docs/contributing/environment.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ noindex: true
sidebar_order: 1
---

This setup assumes you have [pnpm][pnpm], [Volta][volta] and [pre-commit][pre-commit] installed.
This setup assumes you have [pnpm][pnpm], [Volta][volta] and [prek][prek] installed.

If you prefer not to use Volta, make sure you're using Node 20.
If you prefer not to use Volta, make sure you're using Node 22.

First, open a terminal and clone the `sentry-docs` repo:

Expand Down Expand Up @@ -43,7 +43,7 @@ You will now be able to access docs via http://localhost:3000.

[Next.js]: https://nextjs.org/
[volta]: https://volta.sh/
[pre-commit]: https://pre-commit.com/
[prek]: https://prek.j178.dev/installation/
[pnpm]: https://pnpm.io/

## Linting
Expand All @@ -66,11 +66,11 @@ pnpm lint:prettier:fix

We use [typos](https://github.com/crate-ci/typos) to check for spelling errors.

**Option 1: Via pre-commit**
**Option 1: Via prek**

```bash
make develop # Sets up pre-commit hooks
pre-commit run typos --all-files
prek run typos --all-files
```

**Option 2: Direct command**
Expand Down
Loading