Skip to content

Commit 084cf9d

Browse files
committed
Merge branch 'dev' into chore/authentication/deep-link-login
2 parents 96e54e5 + 05db1c0 commit 084cf9d

7 files changed

Lines changed: 86 additions & 10 deletions

File tree

.claude/skills/code-standards/SKILL.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ user-invocable: false
2929

3030
**Exception:** serialized JSON DTO fields keep their wire-format casing — see *Serialized JSON DTOs* below.
3131

32+
**Namespaces are domain names, not folder paths** — never suppress a folder-namespace mismatch with `// ReSharper disable once CheckNamespace`; fix the namespace or leave the warning visible. Full rule: `docs/code-style-guidelines.md` § Namespaces.
33+
3234
## Member Ordering
3335

3436
Within a class, group members in this order:

.github/workflows/pr-comment-warnings.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ jobs:
7373
{
7474
echo "body<<EOF"
7575
echo "<!-- warning-ratchet -->"
76+
blocked=false
7677
if [ -z "$BASELINE" ]; then
7778
echo "**Warnings counted: $COUNT** (no baseline established yet)"
7879
elif [ "$COUNT" -lt "$BASELINE" ]; then
@@ -81,6 +82,27 @@ jobs:
8182
echo "**Warnings unchanged: $BASELINE => $COUNT** — allowed on release/hotfix branches."
8283
else
8384
echo "**Warnings not reduced: $BASELINE => $COUNT** — remove at least one warning to merge."
85+
blocked=true
86+
fi
87+
88+
# Always list the warnings/errors in files this PR changed; expand only when blocking.
89+
total=$(jq -r '.pr_findings_total // 0' warning-result.json)
90+
if [ "$total" -gt 0 ]; then
91+
open_attr=""
92+
[ "$blocked" = "true" ] && open_attr=" open"
93+
echo ""
94+
echo "<details$open_attr><summary>Warnings/errors in files changed by this PR ($total)</summary>"
95+
echo ""
96+
echo "| File | Line | Rule | Message |"
97+
echo "| --- | --- | --- | --- |"
98+
jq -r '.pr_findings[] | "| \(.file) | \(.line) | \(.rule) | \(.message | gsub("[|\r\n]"; " ")) |"' warning-result.json
99+
shown=$(jq -r '.pr_findings | length' warning-result.json)
100+
if [ "$total" -gt "$shown" ]; then
101+
echo ""
102+
echo "_…and $((total - shown)) more (see the \`csharp-lint-reports\` artifact)._"
103+
fi
104+
echo ""
105+
echo "</details>"
84106
fi
85107
echo "EOF"
86108
} >> "$GITHUB_OUTPUT"

.github/workflows/test.yml

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,44 @@ jobs:
398398
aws s3 cp baseline.json "s3://$EXPLORER_TEAM_S3_BUCKET/${{ env.WARNINGS_BASELINE_S3_KEY }}" \
399399
--content-type application/json --cache-control no-cache
400400
401+
# Warnings/errors located in files this PR changed — surfaced in the PR comment.
402+
- name: Collect PR-file warnings
403+
if: ${{ !cancelled() && github.event_name == 'pull_request' }}
404+
id: pr-findings
405+
env:
406+
BASE_REF: ${{ github.event.pull_request.base.ref }}
407+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
408+
MAX_FINDINGS: 50 # max rows listed in the PR comment; change here
409+
run: |
410+
set -euo pipefail
411+
echo '[]' > pr-findings.json
412+
echo "total=0" >> "$GITHUB_OUTPUT"
413+
[ -f filtered.json ] || exit 0
414+
415+
base=$(git merge-base "origin/$BASE_REF" "$HEAD_SHA" 2>/dev/null || true)
416+
[ -n "$base" ] || { echo "No merge-base - skipping PR-file findings."; exit 0; }
417+
418+
# repo-relative paths -> project-relative (strip leading 'Explorer/')
419+
git diff --name-only "$base" "$HEAD_SHA" -- '*.cs' \
420+
| sed 's#^Explorer/##' | sort -u > changed-files.txt
421+
422+
jq --rawfile changed changed-files.txt '
423+
($changed | split("\n") | map(select(length > 0))) as $files
424+
| map({
425+
file: (.locations[0].physicalLocation.artifactLocation.uri // ""),
426+
line: (.locations[0].physicalLocation.region.startLine // 0),
427+
rule: (.ruleId // ""),
428+
level: (.level // "warning"),
429+
message: (.message.text // "")
430+
})
431+
| map(select(.file as $f | $files | index($f) != null))
432+
' filtered.json > pr-findings-all.json
433+
434+
total=$(jq length pr-findings-all.json)
435+
jq --argjson max "$MAX_FINDINGS" '.[:$max]' pr-findings-all.json > pr-findings.json # cap comment size
436+
echo "total=$total" >> "$GITHUB_OUTPUT"
437+
echo "PR-file findings: $total (listing up to $MAX_FINDINGS)"
438+
401439
# Hand the numbers to the trusted workflow_run companion that posts the PR comment.
402440
# Runs even when the gate above failed (!cancelled), so a failing run still refreshes the comment
403441
# instead of leaving a stale message.
@@ -416,7 +454,9 @@ jobs:
416454
--argjson count "$COUNT" \
417455
--arg baseline "$BASELINE" \
418456
--argjson allow_equal "$IS_RELEASE_OR_HOTFIX" \
419-
'{pr: $pr, count: $count, baseline: (if $baseline == "" then null else ($baseline | tonumber) end), allow_equal: $allow_equal}' \
457+
--slurpfile findings pr-findings.json \
458+
--argjson findings_total "${{ steps.pr-findings.outputs.total || 0 }}" \
459+
'{pr: $pr, count: $count, baseline: (if $baseline == "" then null else ($baseline | tonumber) end), allow_equal: $allow_equal, pr_findings: ($findings[0] // []), pr_findings_total: $findings_total}' \
420460
> warning-result.json
421461
cat warning-result.json
422462

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ Reviewers have repeatedly identified AI-generated code by these smells. Check yo
131131
* **Wiring pooled/virtualized list items per rebind.** For item pools, wire callbacks once when the item is created, not every time `SetItemData` runs. Prefer an `Action` field (single subscriber, direct assignment) over C# `event` (`+=`/`-=` churn) when there is exactly one subscriber.
132132
* **Reimplementing primitives that already exist.** Before writing manual atlas UV math, check `TMP_Sprite Asset`. Before hand-batching profile lookups, check the batched `GetProfilesAsync(IReadOnlyList<string>, ct)` overload. Before adding a bespoke event pathway, check `ViewEventBus` / `ChatEvents`.
133133
* **Comments that narrate caller/external behavior.** A comment must state only what the annotated code itself does or guarantees ("remove the corrupt file so the next read doesn't hit it"), never what callers or upper layers will do with the result ("so callers treat it as a miss and re-download"). External behavior can change without this code changing, silently turning the comment into a lie.
134+
* **Suppressing `CheckNamespace` with a ReSharper comment.** Never add `// ReSharper disable once CheckNamespace` (or the file-wide variant) — fix the namespace or leave the warning visible. Rationale and full rule: [`docs/code-style-guidelines.md` § Namespaces](docs/code-style-guidelines.md#namespaces).
134135

135136
### Other project-specific rules
136137

Explorer/Assets/DCL/Infrastructure/SceneLifeCycle/Systems/UpdateCurrentSceneSystem.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ namespace ECS.SceneLifeCycle.Systems
2121
[UpdateInGroup(typeof(RealmGroup))]
2222
public partial class UpdateCurrentSceneSystem : BaseUnityLoopSystem
2323
{
24+
2425
private const string NO_DATA_STRING = "<No data>";
2526

2627
private static readonly int SRC_BLEND = Shader.PropertyToID("_SrcBlend");

docs/code-style-guidelines.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,10 @@ List<string> filteredWords = new FilterLogic(listWords).
248248
- Folder structure should be aligned with the namespaces.
249249
- Not every folder should be namespace provider, especially folders like `Scripts`, `MainScripts`, `Assets`.
250250
- Folders that are deep in the folders hierarchy should be without namespace.
251+
- Never suppress the folder-namespace inspection with `// ReSharper disable once CheckNamespace` (or the file-wide `// ReSharper disable CheckNamespace`).
252+
- Namespaces name domains and deliberately survive folder and assembly reshuffles (e.g. `DCL.Ipfs` lives in the `DCL.Network` assembly), so a folder-namespace mismatch is often intentional.
253+
- The lint filter shared by CI and the local hook (`scripts/lint/filter-warnings.sh`) already excludes the `CheckNamespace` inspection, so the comment changes nothing in the warning count — it is pure noise, and it hides genuine cases where a type joined the wrong namespace.
254+
- If the IDE flags a mismatch, either move the type into the domain namespace its closest collaborators live in, or leave the warning visible.
251255

252256
### Whitespaces
253257

scripts/lint/filter-warnings.sh

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
#!/usr/bin/env bash
22
# Filter false positives out of an InspectCode report and print the remaining count.
3-
# Excludes:
4-
# - '.CSharpErrors' / '.CppCompilerErrors': false positives from '--no-build' (unresolved refs).
5-
# - vendored / third-party code we don't own: everything under 'Packages/' and the
6-
# 'DOTween' / 'SocketIO' plugins.
7-
# - everything below warning severity (SARIF level 'note' = ReSharper suggestions/hints):
8-
# only warning-or-higher results participate in the ratchet.
3+
# Only warning-or-higher results (SARIF level 'warning'/'error') in code we own
4+
# participate in the ratchet; the exclusions and their reasons are declared inline below.
95
#
106
# Usage: filter-warnings.sh <report.json> <filtered_output.json>
117
# Writes the filtered results array to <filtered_output.json>; prints the integer count to stdout.
@@ -14,18 +10,28 @@ set -euo pipefail
1410
report="${1:?usage: filter-warnings.sh <report.json> <filtered_output.json>}"
1511
out="${2:?usage: filter-warnings.sh <report.json> <filtered_output.json>}"
1612

13+
excluded_rules=(
14+
'.CSharpErrors' # false positive from '--no-build' (unresolved refs)
15+
'.CppCompilerErrors' # false positive from '--no-build' (unresolved refs)
16+
'CheckNamespace' # namespaces are domain names, not folder paths (docs/code-style-guidelines.md § Namespaces)
17+
)
18+
19+
# vendored / third-party code we don't own
20+
excluded_paths='^(Packages/|Assets/Plugins/(DOTween|SocketIO)/)'
21+
1722
if [ ! -f "$report" ]; then
1823
echo "filter-warnings: report not found at '$report'" >&2
1924
exit 1
2025
fi
2126

22-
jq '
27+
jq --argjson excludedRules "$(printf '%s\n' "${excluded_rules[@]}" | jq -R . | jq -s .)" \
28+
--arg excludedPaths "$excluded_paths" '
2329
.runs[0].results
2430
| map(select(
2531
((.level // "warning") | IN("warning", "error"))
26-
and (.ruleId != ".CSharpErrors" and .ruleId != ".CppCompilerErrors")
32+
and ((.ruleId // "") | IN($excludedRules[]) | not)
2733
and ((.locations[0].physicalLocation.artifactLocation.uri // "")
28-
| test("^(Packages/|Assets/Plugins/(DOTween|SocketIO)/)"; "i") | not)
34+
| test($excludedPaths; "i") | not)
2935
))
3036
' "$report" > "$out"
3137

0 commit comments

Comments
 (0)