Add comprehensive style guidelines to STYLE.md#282
Add comprehensive style guidelines to STYLE.md#282oclivegriffin wants to merge 1 commit intomainfrom
Conversation
Expand coding style guide with additional sections on: - Pure functions and I/O handling - Data structures (avoid bare dicts for heterogeneous data) - Type invariants and encoding them in types - Distinguishing None from empty collections - Function design and default argument guidelines - Code hygiene (deleting unused code, removing unnecessary logs) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Lgtm. Some things that are included in https://github.com/goodfire-ai/spd/pull/275/files but not in your changes (which may be intentional but flagging in case not):
and some other things that probably aren't worth flagging. |
|
Sorry yea wasn't trying to be a replacement of your PR, just a quick PR to integrate my "SWE principles" snippet into this |
|
Edit: Actually oops ignore this - I thought you were responding to another PR change. Original: Lol no dw I was just pushing this because it was sitting in my unstaged files and wanted to clean things up. I have mostly parked this for the time being - use whatever you feel like! |
|
@ocg-goodfire The STYLE.md file was deleted in PR332 in favour of just putting style comments in the CLAUDE.md file. Up to you if you want to include these changes in the new CLAUDE.md or re-introduce the STYLE.md file. |
|
(This PR should probably be closed - will leave this up to @ocg-goodfire) |
4 participants
Description
Expand the STYLE.md code style guide with additional sections covering:
Related Issue
N/A
Motivation and Context
The existing STYLE.md was fairly sparse. These additions document coding principles and conventions that help maintain code quality and consistency across the codebase. The guidelines emphasize safety, fail-fast behavior, and making invalid states unrepresentable through types.
How Has This Been Tested?
Documentation-only change - no testing required.
Does this PR introduce a breaking change?
No - this is a documentation update only.
🤖 Generated with Claude Code