Address review feedback: remove emojis, enforce sentence case for headers#29
Address review feedback: remove emojis, enforce sentence case for headers#29
Conversation
Co-authored-by: Eoic <18404082+Eoic@users.noreply.github.com>
Co-authored-by: Eoic <18404082+Eoic@users.noreply.github.com>
Co-authored-by: Eoic <18404082+Eoic@users.noreply.github.com>
Co-authored-by: Eoic <18404082+Eoic@users.noreply.github.com>
.github/AI_CODE_STYLE_GUIDE.md
Outdated
| return subtotal + tax; | ||
| } | ||
|
|
||
| // ✅ Good - self-documenting code with clear names |
There was a problem hiding this comment.
Do not use emojis anywhere in the code or the docs. Also add instructions in this file that emojis are not allowed anywhere at all.
.github/AI_CODE_STYLE_GUIDE.md
Outdated
| return subtotal + tax; | ||
| } | ||
|
|
||
| // ✅ Good - complex logic with helpful comment |
.github/AI_CODE_STYLE_GUIDE.md
Outdated
|
|
||
| --- | ||
|
|
||
| ## Naming Conventions |
There was a problem hiding this comment.
Prefer using capitalization style that only capitalizes the first word in a title, for example, prefer using "Naming conventions" instead of "Naming Conventions". Add instructions about that and update this document to use the said style.
… sentence case Co-authored-by: Eoic <18404082+Eoic@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds an AI-focused code style guide to the repo and links it from the main README, aiming to standardize formatting rules and documentation style (including emoji removal/prohibition and sentence-case headers).
Changes:
- Add a new
.github/AI_CODE_STYLE_GUIDE.mddocumenting repo-aligned TypeScript/ESLint conventions and documentation standards. - Add an AI-agent pointer link in
README.mdto the new style guide.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| README.md | Adds a link directing AI agents to the new style guide. |
| .github/AI_CODE_STYLE_GUIDE.md | Introduces a comprehensive AI code style guide, including emoji prohibition and sentence-case header rules. |
| ### Variables and Functions | ||
| - Use **camelCase** for variables and functions: `animationSpeed`, `createRenderer` | ||
|
|
||
| ### Private members | ||
| - Prefix private class members with underscore: `_state`, `_renderer`, `_permutations` |
There was a problem hiding this comment.
This heading uses Title Case ("Variables and Functions") even though the guide mandates sentence case for all headers. Rename it to sentence case (and scan nearby headings like "Getters and Setters" for the same issue).
|
|
||
| ### Export patterns | ||
|
|
||
| #### Grouped Exports |
There was a problem hiding this comment.
This subheading is still Title Case ("Grouped Exports"), which conflicts with the guide’s sentence-case rule for headers. Convert it (and the nearby "Direct Exports") to sentence case for consistency.
| #### Grouped Exports | |
| #### Grouped exports |
| ├── patterns/ # Pattern implementations | ||
| │ ├── pattern.ts # Base pattern class | ||
| │ ├── perlin-noise-pattern.ts | ||
| │ ├── rain-pattern.ts | ||
| │ └── static-noise-pattern.ts | ||
| ├── rendering/ # Rendering implementations | ||
| │ ├── ascii-renderer.ts | ||
| │ └── renderer.ts | ||
| ├── utils/ # Utility functions | ||
| │ └── seeded-random.ts | ||
| ├── plugins/ # Build plugins | ||
| └── __tests__/ # Test files |
There was a problem hiding this comment.
The documented src/ directory tree doesn’t match the repo (e.g., src/demo/ exists). Update this tree snippet so it reflects the actual src/ structure.
| ├── patterns/ # Pattern implementations | |
| │ ├── pattern.ts # Base pattern class | |
| │ ├── perlin-noise-pattern.ts | |
| │ ├── rain-pattern.ts | |
| │ └── static-noise-pattern.ts | |
| ├── rendering/ # Rendering implementations | |
| │ ├── ascii-renderer.ts | |
| │ └── renderer.ts | |
| ├── utils/ # Utility functions | |
| │ └── seeded-random.ts | |
| ├── plugins/ # Build plugins | |
| └── __tests__/ # Test files | |
| ├── demo/ # Demo site source (templates, entrypoints) | |
| ├── patterns/ # Pattern implementations | |
| │ ├── pattern.ts # Base pattern class | |
| │ ├── perlin-noise-pattern.ts | |
| │ ├── rain-pattern.ts | |
| │ ├── static-noise-pattern.ts | |
| │ └── dummy-pattern.ts | |
| ├── rendering/ # Rendering implementations | |
| │ ├── ascii-renderer.ts | |
| │ └── renderer.ts | |
| ├── utils/ # Utility functions | |
| │ └── seeded-random.ts | |
| ├── plugins/ # Build plugins | |
| │ └── eta-plugin.ts | |
| └── __tests__/ # Test files | |
| ├── test-setup.ts |
| * Update pattern state based on context. | ||
| * @param context - The current pattern context. | ||
| */ | ||
| public abstract update(context: PatternContext): this; |
There was a problem hiding this comment.
This abstract Pattern example uses update(...): this, but the actual base class in the repo defines update(...): Pattern (src/patterns/pattern.ts). Align this example signature with the real public API to avoid confusing contributors implementing new patterns.
| public abstract update(context: PatternContext): this; | |
| public abstract update(context: PatternContext): Pattern; |
|
|
||
| ### Emojis and visual markers | ||
| - **Never use emojis** in code, comments, documentation, or commit messages | ||
| - Do not use visual markers like ❌, ✅, 🚀, etc. |
There was a problem hiding this comment.
The "Emojis and visual markers" section prohibits emojis in documentation, but it currently includes emoji examples (❌/✅/🚀). This contradicts the rule and also violates the stated prohibition. Replace the emoji characters with plain-text descriptions (e.g., "x-mark", "checkmark") or remove the examples entirely.
| - Do not use visual markers like ❌, ✅, 🚀, etc. | |
| - Do not use visual markers like "x-mark", "checkmark", "rocket" icons, etc. |
| @@ -0,0 +1,911 @@ | |||
| # AI Code Style Guidelines for ASCIIGround | |||
There was a problem hiding this comment.
The document title is not in sentence case (it uses Title Case), but the guide mandates sentence case for all titles/headers. Update this top-level heading to sentence case to match the standard.
| # AI Code Style Guidelines for ASCIIGround | |
| # AI code style guidelines for ASCIIGround |
Addresses three review comments on the AI code style guide: emoji removal, emoji prohibition guidelines, and header capitalization standardization.
Changes
Removed emojis from code examples: Replaced
// ❌ Badand// ✅ Goodwith plain text// Badand// GoodAdded style enforcement sections:
Converted all headers to sentence case: Updated 60+ section headers from Title Case to sentence case
Updated summary checklist: Added "No emojis" and "Use sentence case for titles" items
Before/After
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.