docs: add contributor guidelines - #397
Conversation
There was a problem hiding this comment.
Generated by Everglow Holistic Review for #397 · ⊞ 25.3K
There was a problem hiding this comment.
Generated by Everglow Holistic Review for #397 · ⊞ 20.9K
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Holistic Review
Motivation: Well-justified. The repository was transferred from Solaestas/Everglow to CycloneClub/Everglow and needed updated branding, community health files (CoC, contributing guide), and a more navigable AGENTS.md structure that frontloads task-to-document references for AI agents.
Approach: Reasonable. The AGENTS.md restructuring condenses content into a "Start Here" task table + concise topical sections, redirecting detail to existing per-system docs. README.md gets corrected repo URLs and a cleaned-up description. New CODE_OF_CONDUCT.md and CONTRIBUTING.md follow standard open-source conventions. Yggdrasil/AGENTS.md accurately maps the module's directory structure and constraints.
Summary:
Detailed Findings
✅ Documentation Quality — Well-structured and consistent
All four commits are clean, focused, and logically ordered. The new community files (CODE_OF_CONDUCT.md, CONTRIBUTING.md) follow standard open-source templates and correctly link to each other and to AGENTS.md. README.md changes correctly update all Solaestas/Everglow references to CycloneClub/Everglow and replace the casual description with a more professional project overview. The old strikethrough on the art-assets license paragraph was properly removed (it was confusing before).
✅ Yggdrasil Module Guidelines — Accurate
The new Sources/Modules/Yggdrasil/AGENTS.md accurately reflects the module's actual structure:
- All 11 subdirectories listed in the module map exist and match
- Cross-module dependencies (CagedDomain, Food, SpellAndSkull, SubSpace) were verified against
Everglow.Yggdrasil.csproj - Key files (
YggdrasilWorld.cs,YggdrasilModule.cs,Common/YggdrasilPlayer.cs,Netcode/) all exist - The README.md it references also exists
- Constraints about
!Main.dedServguards,IOcclusionProjectile, packet synchronization, and resource immutability are consistent with root AGENTS.md conventions
✅ AGENTS.md Restructuring — Critical content preserved
Despite the 215-line removal / 108-line addition reshape, the restructured file preserves all core technical information:
- Build prerequisites and commands (with CI-equivalent flags)
- Architecture layering (Core → Function → Modules → Everglow)
- Auto-loading via
AddContents()with no manual calls ModAssetsource-gen path conventions- Resource packing whitelist rules
- Localization constraints (en-US + zh-Hans mandatory, keys additive only)
- Unit testing rules (
Program.SavePath, nonew Main(), reflection fortalkNPC) - UTF-8 BOM verification command
- The full 9-rule agent conduct guidelines
💡 Title Change — AGENTS.md H1 differs from filename
The old AGENTS.md used # AGENTS.md as its H1. The new version uses # Repository Guidelines. While this is a style choice, it diverges from the file's own name and from the convention used by Yggdrasil/AGENTS.md (which uses # Yggdrasil Module Guidelines). Consider either reverting to # AGENTS.md or keeping a secondary reference to the filename for discoverability.
💡 Lost Module Inventory — Active module names no longer enumerated
The old AGENTS.md explicitly listed all 14 active modules (AssetReplace, CagedDomain, EternalResolve, Example, Food, MEAC, Minortopography, Myth, Ocean, Plant, PlantAndFarm, SpellAndSkull, SubSpace, Yggdrasil) and the 3 retired ones. The new version says "14 active modules" without enumerating them. An agent tasked with "add content to a module" must now consult Directory.Build.props to discover which modules exist. This is a minor discoverability regression.
[!NOTE] This review was generated by this repository's Holistic Review agentic workflow to complement human review.
Generated by Everglow Holistic Review for #397 · ⊞ 13.7K
There was a problem hiding this comment.
Holistic Review
Motivation: Well-justified and unchanged since the prior review. The repository transfer from Solaestas/Everglow to CycloneClub/Everglow needed updated branding, community health files, and a restructured AGENTS.md. The incremental README description reversion is a stylistic preference from the PR author — restoring the original casual tone over the professional description introduced earlier in this PR.
Approach: Reasonable and consistent. The sole incremental change (commit 881a710) reverts a single README sentence to the pre-PR text. All other files (AGENTS.md, CODE_OF_CONDUCT.md, CONTRIBUTING.md, Yggdrasil/AGENTS.md) are unchanged from the prior reviewed state.
Summary:
- review 4851283661 — commit
11487d5, verdict was⚠️ Needs Human Review, now⚠️ Needs Human Review; assessment unchanged.
Detailed Findings
✅ README Reversion — Purely cosmetic, no regressions
The new commit reverts the README description from "Everglow is a content-focused Terraria mod in active development." back to the original "Everglow is a freaking awesome mod for Terraria!." This is the PR author's own suggestion applied to their own branch — no review action needed. All other README changes (corrected GitHub URLs from Solaestas to CycloneClub, updated section headers, compatibility and contributing sections) remain intact.
💡 Pre-existing Typo — Trailing period after exclamation mark
The restored text reads Terraria!. — a redundant period after an exclamation mark. This typo existed before the PR and was reintroduced by the reversion. It does not appear in the base-to-head diff (the line matches base), so it cannot be flagged as an inline suggestion. Consider fixing to Terraria! in a follow-up.
[!NOTE] This review was generated by this repository's Holistic Review agentic workflow to complement human review.
Generated by Everglow Holistic Review for #397 · ⊞ 13.7K
No description provided.