Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ dotnet_diagnostic.IL2026.severity = none
dotnet_diagnostic.IL2070.severity = none
dotnet_diagnostic.IL2075.severity = none
dotnet_diagnostic.IL2090.severity = none
dotnet_diagnostic.CA2255.severity = none

# This appears to be broken and results in false positives (causing dotnet format to delete valid test scenarios)
dotnet_diagnostic.xUnit1025.severity = none
22 changes: 22 additions & 0 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash
Comment thread
cursor[bot] marked this conversation as resolved.
set -e
Comment thread
cursor[bot] marked this conversation as resolved.

echo "🔍 Checking code formatting..."

dotnet format Sentry.slnx --no-restore \
--exclude ./modules ./**/*OptionsSetup.cs ./test/Sentry.Tests/AttributeReaderTests.cs > /dev/null 2>&1

Check failure on line 7 in .githooks/pre-commit

View check run for this annotation

@sentry/warden / warden: code-review

Pre-commit hook auto-formats unstaged files and may modify user's working tree

The hook runs `dotnet format` (without `--verify-no-changes`), which mutates files in the working tree rather than just verifying. Combined with `set -e`, if formatting itself fails the hook aborts; but more critically, it formats the entire solution including unstaged changes the developer did not intend to commit. This causes side effects outside the commit's scope and can corrupt in-progress work, contradicting the PR description which states the hook should run `dotnet format --verify-no-changes` (check-only mode).

Check warning on line 7 in .githooks/pre-commit

View check run for this annotation

@sentry/warden / warden: code-review

`set -e` causes the hook to exit silently when `dotnet format` returns non-zero

With `set -e` at the top of the script, any non-zero exit from `dotnet format` (e.g., when `--verify-no-changes` is added, or when the tool encounters an error) will terminate the script immediately, bypassing the helpful error message block below. The user will see only the initial "🔍 Checking code formatting..." line and a failed commit with no explanation of how to fix it.

Check failure on line 7 in .githooks/pre-commit

View check run for this annotation

@sentry/warden / warden: find-bugs

[BAN-HPT] Pre-commit hook modifies unstaged working-tree files and conflates them with staged changes (additional location)

The hook runs `dotnet format` against the entire working tree (not just staged content), then uses `git diff` (working tree vs index) to detect changes. This means: (1) it silently rewrites unstaged files the developer did not intend to commit, and (2) the failure path tells the developer to run `git add -u`, which would stage unrelated unstaged modifications along with formatting fixes. A developer with in-progress work in other files will have those files reformatted and potentially committed unintentionally.

Check failure on line 7 in .githooks/pre-commit

View check run for this annotation

@sentry/warden / warden: find-bugs

[BAN-HPT] Pre-commit hook modifies unstaged working-tree files and conflates them with staged changes (additional location)

The hook runs `dotnet format` against the entire working tree (not just staged content), then uses `git diff` (working tree vs index) to detect changes. This means: (1) it silently rewrites unstaged files the developer did not intend to commit, and (2) the failure path tells the developer to run `git add -u`, which would stage unrelated unstaged modifications along with formatting fixes. A developer with in-progress work in other files will have those files reformatted and potentially committed unintentionally.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Outdated
Comment thread
jamescrosswell marked this conversation as resolved.
Outdated
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Outdated
Comment thread
jamescrosswell marked this conversation as resolved.
Outdated

if ! git diff --quiet; then

Check failure on line 9 in .githooks/pre-commit

View check run for this annotation

@sentry/warden / warden: code-review

[PHR-3M5] Pre-commit hook auto-formats unstaged files and may modify user's working tree (additional location)

The hook runs `dotnet format` (without `--verify-no-changes`), which mutates files in the working tree rather than just verifying. Combined with `set -e`, if formatting itself fails the hook aborts; but more critically, it formats the entire solution including unstaged changes the developer did not intend to commit. This causes side effects outside the commit's scope and can corrupt in-progress work, contradicting the PR description which states the hook should run `dotnet format --verify-no-changes` (check-only mode).
echo ""
echo "❌ Code formatting issues found!"
echo ""
echo "Please stage the formatting fixes and commit again:"
echo ""
echo " git add -u"
Comment thread
sentry-warden[bot] marked this conversation as resolved.
echo " git commit"

Check failure on line 16 in .githooks/pre-commit

View check run for this annotation

@sentry/warden / warden: find-bugs

Pre-commit hook modifies unstaged working-tree files and conflates them with staged changes

The hook runs `dotnet format` against the entire working tree (not just staged content), then uses `git diff` (working tree vs index) to detect changes. This means: (1) it silently rewrites unstaged files the developer did not intend to commit, and (2) the failure path tells the developer to run `git add -u`, which would stage unrelated unstaged modifications along with formatting fixes. A developer with in-progress work in other files will have those files reformatted and potentially committed unintentionally.
echo ""
exit 1
fi

echo "✅ Code formatting looks good!"
exit 0
18 changes: 18 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,24 @@ For a big feature it's advised to raise an issue to discuss it first.
* To quickly get up and running, you can just run `dotnet build SentryNoMobile.slnf` (you're skipping the mobile targets)
* To run a full build in Release mode and test, before pushing, run `./build.sh` or `./build.cmd`

## Git Hooks (Optional but Recommended)

To automatically verify code formatting before committing, you can set up a pre-commit hook:

```bash
./scripts/setup-hooks.sh
```
Comment thread
sentry[bot] marked this conversation as resolved.

This configures git to run `dotnet format --verify-no-changes` before each commit. If formatting issues are found, the commit will be prevented and you'll need to run:
Comment thread
sentry[bot] marked this conversation as resolved.
Outdated

```bash
dotnet format Sentry.slnx --no-restore --exclude ./modules ./**/*OptionsSetup.cs ./test/Sentry.Tests/AttributeReaderTests.cs
```

Then stage the formatting changes and commit again. This helps catch formatting issues early and reduces CI failures.

**Note:** You can bypass the hook for a specific commit using `git commit --no-verify` if needed.

## Minimal Dependencies

* The latest versions of the following .NET SDKs:
Expand Down
14 changes: 14 additions & 0 deletions scripts/setup-hooks.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash
set -e

echo "Setting up git hooks..."

# Configure git to use .githooks directory for hooks
git config core.hooksPath .githooks

echo ""
echo "✅ Git hooks configured successfully!"
echo ""
echo "The pre-commit hook will now verify code formatting before each commit."
echo "To bypass the hook for a specific commit, use: git commit --no-verify"
echo ""
Loading