Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
52 changes: 52 additions & 0 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#!/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..."

STASH_NAME="pre-commit-$(date +%s)"

# Runs on any exit — ensures stashed changes are always restored
restore_stash() {
if git stash list | grep -q "$STASH_NAME"; then
git stash pop 2>/dev/null || {
echo ""
echo "⚠️ Your unstaged changes could not be restored automatically (stash conflict)."
echo " Run 'git stash pop' to restore them manually."
}
fi

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

View check run for this annotation

@sentry/warden / warden: find-bugs

Stash detection by message can match unrelated stashes and miss empty stashes

`git stash push --keep-index` creates no stash entry when there are no unstaged changes (and exits 0 due to `|| true`). The later `git stash list | grep -q "$STASH_NAME"` correctly skips popping in that case, but the pattern match is a substring search against `git stash list` output, which could be matched by an unrelated user stash whose message happens to contain `pre-commit-<timestamp>` (unlikely but possible). More importantly, on a stash-conflict pop failure, the script tells the user to run `git stash pop`, but does not identify which stash to pop — the user may have multiple stashes and pop the wrong one.
}
trap restore_stash EXIT

# Stash unstaged changes (keep staged changes in working tree)
# This ensures we only check formatting on what's being committed
git stash push --keep-index --quiet --message "$STASH_NAME" || true

# Run dotnet format (matches CI command exactly)
dotnet format Sentry.slnx --no-restore \
--exclude ./modules ./**/*OptionsSetup.cs ./test/Sentry.Tests/AttributeReaderTests.cs > /dev/null 2>&1

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

View check run for this annotation

@sentry/warden / warden: code-review

Hook runs `dotnet format` in write mode despite claiming check-only behavior

The PR description and hook comment state this is a check-only verification (matching CI's `--verify-no-changes`), but the actual command omits `--verify-no-changes` and runs `dotnet format` in its default write mode, which modifies files in the working tree. After the failed commit, the working tree contains auto-applied formatting changes the user never asked for. This contradicts the documented design and produces side effects on the developer's working tree on every formatting failure.

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

View check run for this annotation

@sentry/warden / warden: find-bugs

Pre-commit hook silently modifies staged files via `dotnet format`

`dotnet format` (without `--verify-no-changes`) rewrites files on disk. Because `git stash push --keep-index` leaves the staged content in the working tree, the format command modifies those staged files. The hook then detects the diff, aborts the commit, and the trap pops the stash — but the formatter's modifications to the working tree are NOT reverted. The developer's working copy now contains formatter changes they never asked for, mixed with their original edits, and on the next `git add -u`/commit they will commit formatter output silently. The PR description claims the hook uses `--verify-no-changes` (check-only), but the actual script does not.

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

View check run for this annotation

@sentry/warden / warden: find-bugs

[96L-H56] Pre-commit hook silently modifies staged files via `dotnet format` (additional location)

`dotnet format` (without `--verify-no-changes`) rewrites files on disk. Because `git stash push --keep-index` leaves the staged content in the working tree, the format command modifies those staged files. The hook then detects the diff, aborts the commit, and the trap pops the stash — but the formatter's modifications to the working tree are NOT reverted. The developer's working copy now contains formatter changes they never asked for, mixed with their original edits, and on the next `git add -u`/commit they will commit formatter output silently. The PR description claims the hook uses `--verify-no-changes` (check-only), but the actual script does not.
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

# Check if dotnet format made any changes
FORMAT_CHANGED=false
if ! git diff --quiet; then
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Outdated
FORMAT_CHANGED=true
fi

# If formatting changes were detected, fail the commit
if [ "$FORMAT_CHANGED" = true ]; then
echo ""
echo "❌ Code formatting issues found!"
echo ""
echo "Please run the following command to fix formatting:"
echo ""
echo " dotnet format Sentry.slnx --no-restore --exclude ./modules ./**/*OptionsSetup.cs ./test/Sentry.Tests/AttributeReaderTests.cs"
echo ""
echo "Then stage the changes and commit again:"
echo ""
echo " git add -u"

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

View check run for this annotation

@sentry/warden / warden: code-review

Recovery instructions risk staging unrelated unstaged changes

After a formatting failure, the trap restores the user's previously stashed unstaged changes back into the working tree. The error message then instructs the user to run `git add -u && git commit`, which would stage ALL modified tracked files — including unstaged changes that were never meant to be part of this commit. This silently expands the scope of the user's commit beyond what they originally staged.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
echo " git commit"
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