Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
56 changes: 56 additions & 0 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#!/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."
echo " dotnet format modified lines that overlap with your unstaged edits."
echo " Resolve the conflict markers in the affected files, then run:"
echo ""
echo " git stash drop"
echo ""
}
fi
}
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 \

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

View check run for this annotation

@sentry/warden / warden: find-bugs

dotnet format mutates the working tree, leaving format edits behind on failure and corrupting stash pop

The hook runs `dotnet format` without `--verify-no-changes`, so it actively rewrites files in the working tree instead of just checking them (contradicting the PR's stated 'check-only mode'). When formatting issues exist, the script exits 1 and the EXIT trap runs `git stash pop`, but the working tree now contains dotnet format's edits on top of the staged content. Restoring the previously stashed unstaged changes will frequently conflict, and even when it doesn't, the developer is left with unrequested formatting modifications applied to their working tree (and possibly their stash dropped). On a successful commit the hook also leaves these working-tree mutations in place, since nothing resets them before exit.

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

View check run for this annotation

@sentry/warden / warden: find-bugs

`set -e` causes the hook to abort silently if dotnet format exits non-zero for any reason

With `set -e` enabled at the top of the script, any non-zero exit from `dotnet format` (e.g., build/restore error, invalid argument, dotnet not installed) will terminate the script immediately. The EXIT trap then runs `git stash pop` and the script exits non-zero, blocking the commit without printing the helpful 'formatting issues found' message — and without surfacing the actual dotnet error, since stdout/stderr are redirected to /dev/null. Developers will see a failed commit with no diagnostic output.

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

View check run for this annotation

@sentry/warden / warden: find-bugs

`./**/*OptionsSetup.cs` is expanded by bash before reaching dotnet format, producing wrong --exclude arguments

The script passes `./**/*OptionsSetup.cs` unquoted to `dotnet format`. Bash will perform pathname expansion on this token before invoking dotnet. Without `shopt -s globstar`, `**` behaves like `*` and matches only one directory level; with globstar enabled it expands recursively. Either way, dotnet format receives a list of concrete paths (or the literal pattern if no match) — not the intended glob. As a result the exclude list differs from CI, so the hook may flag (or miss) files that CI treats differently, defeating the 'matches CI command exactly' goal.
--exclude ./modules ./**/*OptionsSetup.cs ./test/Sentry.Tests/AttributeReaderTests.cs > /dev/null 2>&1

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

View check run for this annotation

@sentry/warden / warden: code-review

`set -e` causes script to exit before formatting failure path runs, leaving stash unrestored on dotnet format errors

The script uses `set -e` combined with `git stash push --keep-index`. If `dotnet format` exits non-zero (e.g., build failure, restore issue, or tooling error), `set -e` aborts the script immediately. While the EXIT trap will still fire and attempt to restore the stash, the script never reaches the diff check or the user-facing error message — developers get a silent/cryptic failure with output redirected to `/dev/null 2>&1`, making it impossible to diagnose why the hook failed. Combined with `2>&1 > /dev/null`, real formatter errors are completely swallowed.

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

View check run for this annotation

@sentry/warden / warden: code-review

Unquoted `./**/*OptionsSetup.cs` glob is expanded by bash, not passed to dotnet format

The `--exclude` argument `./**/*OptionsSetup.cs` is unquoted, so bash expands it before passing to `dotnet format`. Without `shopt -s globstar`, `**` behaves as a single `*` and won't recurse; with globstar enabled it expands to a list of paths that are passed as multiple arguments. Either way, the behavior diverges from CI (which presumably passes the literal pattern) and the exclude may not match the intended files, causing the hook to fail commits over files that CI ignores.
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

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

View check run for this annotation

@sentry/warden / warden: code-review

`dotnet format` runs against entire working tree, not just staged changes, so unrelated formatting issues block commits

`git stash push --keep-index` stashes unstaged changes, leaving staged changes in the working tree — but the working tree still contains all other tracked files. `dotnet format` then scans the entire solution and may reformat files that the developer didn't touch (e.g., pre-existing formatting drift on unrelated files). The hook will then fail the commit even though the staged diff is clean, blocking commits for issues outside the developer's change scope.
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"
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