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

STAGED=$(git diff --cached --name-only --diff-filter=ACM | grep '\.cs$' | tr '\n' ' ')

if [ -z "$STAGED" ]; then
echo "✅ No C# files staged."
exit 0
fi

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

View check run for this annotation

@sentry/warden / warden: code-review

`set -e` with `grep` causes hook to fail when no C# files are staged

With `set -e` enabled, the pipeline `git diff ... | grep '\.cs will terminate the script when `grep` finds no matches (exit code 1), before the `[ -z "$STAGED" ]` check runs. As a result, commits containing no C# files will fail the pre-commit hook instead of exiting cleanly with the intended "No C# files staged" message, blocking unrelated commits. | tr ...` will terminate the script when CODE2 finds no matches (exit code 1), before the CODE3 check runs. As a result, commits containing no C# files will fail the pre-commit hook instead of exiting cleanly with the intended "No C# files staged" message, blocking unrelated commits.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Outdated

dotnet format Sentry.slnx --no-restore \
--include $STAGED \

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

View check run for this annotation

@sentry/warden / warden: code-review

Unquoted `$STAGED` breaks on filenames containing spaces

`$STAGED` is built by joining filenames with spaces and passed unquoted to `--include $STAGED`. Any C# file path containing whitespace will be split into multiple arguments, causing `dotnet format` to receive incorrect (likely nonexistent) paths and either skip files silently or error out. While uncommon in C# projects, this is a latent correctness issue.
--exclude ./modules ./**/*OptionsSetup.cs ./test/Sentry.Tests/AttributeReaderTests.cs > /dev/null 2>&1

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

View check run for this annotation

@sentry/warden / warden: find-bugs

Hook formats working-tree files, not the staged content being committed

The hook collects staged `.cs` files via `git diff --cached` but then runs `dotnet format` against the on-disk (working-tree) versions of those files. When a file is partially staged (some hunks staged, others left unstaged), the formatting check is performed against content that is not what will be committed. This can cause the hook to pass commits whose staged content is actually misformatted, or to fail commits whose staged content is fine because of unstaged edits — defeating the hook's purpose.
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 warning on line 17 in .githooks/pre-commit

View check run for this annotation

@sentry/warden / warden: find-bugs

[X85-UCP] Hook formats working-tree files, not the staged content being committed (additional location)

The hook collects staged `.cs` files via `git diff --cached` but then runs `dotnet format` against the on-disk (working-tree) versions of those files. When a file is partially staged (some hunks staged, others left unstaged), the formatting check is performed against content that is not what will be committed. This can cause the hook to pass commits whose staged content is actually misformatted, or to fail commits whose staged content is fine because of unstaged edits — defeating the hook's purpose.
echo ""

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

View check run for this annotation

@sentry/warden / warden: code-review

Hook auto-modifies files, contradicting documented "check-only" design

The PR description states "Hook verifies formatting but doesn't auto-fix (explicit, matches CI behavior)", but the command invokes `dotnet format` without `--verify-no-changes`, which actively rewrites files on disk. The hook then detects the modifications via `git diff --quiet`. This causes unintended side effects: developer's working tree is mutated on every commit attempt, and any pre-existing unstaged edits will be conflated with formatter output, making it hard to recover the original state.
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

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

View check run for this annotation

@sentry/warden / warden: code-review

[CVK-JG6] `set -e` with `grep` causes hook to fail when no C# files are staged (additional location)

With `set -e` enabled, the pipeline `git diff ... | grep '\.cs will terminate the script when `grep` finds no matches (exit code 1), before the `[ -z "$STAGED" ]` check runs. As a result, commits containing no C# files will fail the pre-commit hook instead of exiting cleanly with the intended "No C# files staged" message, blocking unrelated commits. | tr ...` will terminate the script when CODE2 finds no matches (exit code 1), before the CODE3 check runs. As a result, commits containing no C# files will fail the pre-commit hook instead of exiting cleanly with the intended "No C# files staged" message, blocking unrelated commits.
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
7 changes: 7 additions & 0 deletions dev.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ 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("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