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

if ! git diff --quiet; then
echo "⚠️ Skipping format check: unstaged changes present."
echo " Stage or stash all changes before committing to enable the format check."
exit 0
fi

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

View check run for this annotation

@sentry/warden / warden: find-bugs

[PJD-FYR] Pre-commit hook silently auto-formats staged files, modifying tracked content without consent (additional location)

The hook runs `dotnet format` without `--verify-no-changes`, which mutates files in the working tree. Because the hook only checks `git diff` (unstaged changes) afterward, any formatting fixes applied by `dotnet format` will modify the working tree but the original (unformatted) staged content is still what gets committed. The user is told to `git add -u` and recommit, but if the working tree happens to match the index after formatting (e.g., file was already formatted), the hook exits 0 and an unformatted snapshot is committed. This contradicts the PR description which claims 'check-only mode' using `--verify-no-changes`.

INCLUDE_ARGS=()
while IFS= read -r f; do
INCLUDE_ARGS+=(--include "$f")
done < <(git diff --cached --name-only --diff-filter=ACM | grep '\.cs$' || true)

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

View check run for this annotation

@sentry/warden / warden: find-bugs

Filenames with spaces or special characters break the --include argument list

The loop on lines 12-15 reads staged filenames via `git diff --cached --name-only` and passes them to `dotnet format --include`. `git diff --name-only` does not quote special characters by default unless `core.quotePath` is configured, and even with proper reading, filenames with spaces will be passed correctly to `--include` as separate args — but newer git versions or filenames containing quotes/backslashes produce C-style quoted output (e.g., `"foo\tbar.cs"`) which would not match real paths. This can cause the hook to either miss files or fail unexpectedly.
Comment thread
jamescrosswell marked this conversation as resolved.
Comment thread
jamescrosswell marked this conversation as resolved.
Comment thread
jamescrosswell marked this conversation as resolved.

if [ ${#INCLUDE_ARGS[@]} -eq 0 ]; then
echo "✅ No C# files staged."
exit 0
fi

dotnet format Sentry.slnx --no-restore \
"${INCLUDE_ARGS[@]}" \
--exclude ./modules ./**/*OptionsSetup.cs ./test/Sentry.Tests/AttributeReaderTests.cs > /dev/null 2>&1

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

View check run for this annotation

@sentry/warden / warden: code-review

Pre-commit hook auto-formats files instead of verifying, contradicting stated design

Line 22 invokes `dotnet format` without `--verify-no-changes`, so the command writes formatting fixes directly to the developer's working tree rather than just verifying. The PR description explicitly states the hook uses 'Check-only mode: Hook verifies formatting but doesn't auto-fix,' but the implementation does the opposite. This is an unintended side effect: a developer running `git commit` will have their files silently modified on disk, which is surprising behavior and diverges from CI (which uses `--verify-no-changes`).

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

View check run for this annotation

@sentry/warden / warden: code-review

`./**/*OptionsSetup.cs` exclude pattern silently fails to match nested files

The exclude argument on line 24 uses `./**/*OptionsSetup.cs`, which requires bash's `globstar` option to recurse into subdirectories. The script does not call `shopt -s globstar`, so `**` behaves like a single `*` and only matches files one directory deep relative to the CWD. As a result, `*OptionsSetup.cs` files in deeper paths will not be excluded and will be formatted/checked, defeating the intent of the exclusion.

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

View check run for this annotation

@sentry/warden / warden: find-bugs

Pre-commit hook silently auto-formats staged files, modifying tracked content without consent

The hook runs `dotnet format` without `--verify-no-changes`, which mutates files in the working tree. Because the hook only checks `git diff` (unstaged changes) afterward, any formatting fixes applied by `dotnet format` will modify the working tree but the original (unformatted) staged content is still what gets committed. The user is told to `git add -u` and recommit, but if the working tree happens to match the index after formatting (e.g., file was already formatted), the hook exits 0 and an unformatted snapshot is committed. This contradicts the PR description which claims 'check-only mode' using `--verify-no-changes`.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Outdated

if ! git diff --quiet; then
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"
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 dev.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,20 @@ public Task<int> AiUpdateAsync(GlobalOptions options = default!)
return RunStepAsync("npx @sentry/dotagents install", "npx", "@sentry/dotagents install", options.DryRun);
}

[Command("setup-hooks", Description = "Configure git to use the repo's pre-commit hooks from .githooks/.")]
public Task<int> SetupHooksAsync(GlobalOptions options = default!)
{
Console.WriteLine("[dev] Configuring git hooks path to .githooks/");
return RunStepAsync("git config core.hooksPath", "git", "config core.hooksPath .githooks", options.DryRun);
Comment thread
jamescrosswell marked this conversation as resolved.
}

[Command("remove-hooks", Description = "Restore default git hooks behaviour (stops using .githooks/).")]
public Task<int> RemoveHooksAsync(GlobalOptions options = default!)
{
Console.WriteLine("[dev] Restoring default git hooks path");
return RunStepAsync("git config --unset core.hooksPath", "git", "config --unset core.hooksPath", options.DryRun);
}

[Command("nrest", Description = "Restore the default CI solution.")]
public Task<int> SolutionRestoreAsync(
[Argument("solution", Description = "Solution file to restore. Defaults to platform-specific CI solution if omitted.")] string? solution = null,
Expand Down
Loading