|
| 1 | +# Contributing |
| 2 | + |
| 3 | +## Development setup |
| 4 | + |
| 5 | +``` |
| 6 | +git clone https://github.com/agentruntimecontrolprotocol/ruby-sdk |
| 7 | +cd ruby-sdk |
| 8 | +bundle install |
| 9 | +bundle exec rake spec # run tests |
| 10 | +bundle exec rubocop # lint |
| 11 | +bundle exec steep check # type-check |
| 12 | +``` |
| 13 | + |
| 14 | +## Pull requests |
| 15 | + |
| 16 | +- One logical change per PR. |
| 17 | +- All tests pass on the minimum and maximum supported Ruby versions. |
| 18 | +- RuboCop exits 0. |
| 19 | +- `CHANGELOG.md` updated. Breaking changes bump the major version and are |
| 20 | + flagged prominently. |
| 21 | + |
| 22 | +--- |
| 23 | + |
| 24 | +# Idiomatic Ruby Style Guide |
| 25 | + |
| 26 | +Authoritative style for this gem. Optimized for readability, predictable |
| 27 | +public API surface, and Claude Code consumption. |
| 28 | + |
| 29 | +When this guide conflicts with personal taste, this guide wins. When it |
| 30 | +conflicts with RuboCop defaults, this guide wins and the cop gets configured. |
| 31 | + |
| 32 | +--- |
| 33 | + |
| 34 | +## Hard Limits (Non-Negotiable) |
| 35 | + |
| 36 | +- Line length: aspire **80**, hard cap **100**. |
| 37 | +- Method length: **10 lines** (excluding `def`/`end`). |
| 38 | +- Class length: **100 lines** (excluding comments). |
| 39 | +- Module length: **100 lines**. |
| 40 | +- File length: **200 lines**. |
| 41 | +- Cyclomatic complexity: **6** per method. |
| 42 | +- Perceived complexity: **7** per method. |
| 43 | +- ABC size: **17** per method. |
| 44 | +- Block nesting: **3** levels. |
| 45 | +- Method parameters: **4** (use keyword args or a `Data`/`Struct` beyond). |
| 46 | +- Module nesting: **3** levels deep. |
| 47 | + |
| 48 | +When a limit is breached, **refactor — do not configure the linter to allow |
| 49 | +exceptions**. Excess size is a design smell, not a formatting problem. |
| 50 | + |
| 51 | +--- |
| 52 | + |
| 53 | +## File Organization |
| 54 | + |
| 55 | +- One class or module per file. No exceptions for "small helpers". |
| 56 | +- Filename mirrors the constant: `MyGem::HTTPClient` lives at |
| 57 | + `lib/my_gem/http_client.rb`. |
| 58 | +- `lib/my_gem.rb` is the entry point and does nothing but `require` and |
| 59 | + define the top-level module. |
| 60 | +- `lib/my_gem/version.rb` holds `VERSION = "x.y.z"` and nothing else. |
| 61 | +- Use Zeitwerk for autoloading in any non-trivial gem. |
| 62 | +- Group by domain, not by type. Prefer `lib/my_gem/billing/invoice.rb` over |
| 63 | + `lib/my_gem/models/invoice.rb`. Domain folders scale; type folders don't. |
| 64 | + |
| 65 | +## Magic Comments |
| 66 | + |
| 67 | +Every `.rb` file starts with: |
| 68 | + |
| 69 | +```ruby |
| 70 | +# frozen_string_literal: true |
| 71 | +``` |
| 72 | + |
| 73 | +No exceptions. Add it to generators, templates, and rake tasks too. |
| 74 | + |
| 75 | +--- |
| 76 | + |
| 77 | +## Naming |
| 78 | + |
| 79 | +- `snake_case` for methods, variables, and files. |
| 80 | +- `CamelCase` for classes and modules. |
| 81 | +- `SCREAMING_SNAKE_CASE` for constants. |
| 82 | +- Predicate methods end with `?` and return strict booleans where possible. |
| 83 | +- Bang methods (`!`) signal mutation, danger, or raise-on-failure. Always |
| 84 | + pair with a non-bang version unless raising is the only sensible behavior. |
| 85 | +- Avoid `get_` and `set_` prefixes. Use attribute accessors. |
| 86 | +- Spell names out. Abbreviate only when the abbreviation is more recognizable |
| 87 | + than the word (`url`, `http`, `id`, `db`). |
| 88 | +- Boolean attributes read as questions: `active?`, not `is_active`. |
| 89 | +- Collection variables are plural: `users`, not `user_list`. |
| 90 | + |
| 91 | +--- |
| 92 | + |
| 93 | +## Module & Class Design |
| 94 | + |
| 95 | +- Wrap every public symbol in your top-level gem module. |
| 96 | +- Prefer composition over inheritance. Inherit at most one level deep unless |
| 97 | + modeling a genuine is-a hierarchy. |
| 98 | +- Use `Module` for namespacing and stateless helpers. Use `Class` for objects |
| 99 | + that carry state. |
| 100 | +- Mix in `Comparable` / `Enumerable` instead of reimplementing their |
| 101 | + contracts. |
| 102 | +- Freeze public constants: `DEFAULT_TIMEOUT = 30` then `.freeze` mutable |
| 103 | + literals (arrays, hashes, strings without the magic comment). |
| 104 | +- For value objects, prefer `Data.define` (Ruby 3.2+) or `Struct` over |
| 105 | + hand-rolled classes with `attr_reader` boilerplate. |
| 106 | +- A class with only class methods is a module. Convert it. |
| 107 | + |
| 108 | +--- |
| 109 | + |
| 110 | +## Method Design |
| 111 | + |
| 112 | +- One method, one responsibility. If the name needs "and", split it. |
| 113 | +- Required positional args come first, then keyword args. |
| 114 | +- Use keyword arguments when a method has 3+ parameters, **or** any boolean |
| 115 | + parameter (positional booleans are always wrong). |
| 116 | +- Return meaningful values. Avoid returning `self` unless chaining is part |
| 117 | + of the documented public API. |
| 118 | +- No side effects in predicate methods. |
| 119 | +- Methods that can fail in expected ways either return a result object or |
| 120 | + raise a domain-specific error. Do not return `nil` ambiguously. |
| 121 | + |
| 122 | +```ruby |
| 123 | +# Good |
| 124 | +def find_user(id:) |
| 125 | + repo.fetch(id) or raise NotFoundError, "User #{id} not found" |
| 126 | +end |
| 127 | + |
| 128 | +# Bad |
| 129 | +def find_user(id) |
| 130 | + repo.fetch(id) # returns nil on miss — caller has to guess |
| 131 | +end |
| 132 | +``` |
| 133 | + |
| 134 | +--- |
| 135 | + |
| 136 | +## Error Handling |
| 137 | + |
| 138 | +Define an error hierarchy rooted at one class so consumers can rescue a |
| 139 | +single type: |
| 140 | + |
| 141 | +```ruby |
| 142 | +module MyGem |
| 143 | + class Error < StandardError; end |
| 144 | + class ConfigurationError < Error; end |
| 145 | + class APIError < Error; end |
| 146 | + class RateLimitError < APIError; end |
| 147 | + class NotFoundError < APIError; end |
| 148 | +end |
| 149 | +``` |
| 150 | + |
| 151 | +- Every gem-raised error inherits from `MyGem::Error`. |
| 152 | +- Never `rescue Exception`. Never `rescue` bare. |
| 153 | +- Rescue the narrowest class that handles the case. |
| 154 | +- Error messages include actionable context: IDs, URLs, expected vs got. |
| 155 | +- Do not swallow errors silently. If suppression is required, log via a |
| 156 | + configurable logger. |
| 157 | +- Do not raise from initializers unless construction is genuinely |
| 158 | + impossible. |
| 159 | + |
| 160 | +--- |
| 161 | + |
| 162 | +## Public API Discipline |
| 163 | + |
| 164 | +- Mark every method `public`, `private`, or `protected` explicitly in any |
| 165 | + class that has non-public methods. |
| 166 | +- Tag internal-but-reachable methods with `# @api private` (YARD). |
| 167 | +- Public constants are frozen and documented. |
| 168 | +- Keep the public surface small. Each new public method is a permanent |
| 169 | + maintenance commitment. |
| 170 | +- Do not monkey-patch core classes from a published gem. Use refinements |
| 171 | + only when unavoidable, scoped to the smallest file possible. |
| 172 | +- Never modify `Object`, `Kernel`, `Class`, or `Module`. |
| 173 | + |
| 174 | +--- |
| 175 | + |
| 176 | +## Configuration |
| 177 | + |
| 178 | +Single block-based entry point: |
| 179 | + |
| 180 | +```ruby |
| 181 | +MyGem.configure do |c| |
| 182 | + c.api_key = ENV.fetch("MY_GEM_KEY") |
| 183 | + c.timeout = 10 |
| 184 | +end |
| 185 | +``` |
| 186 | + |
| 187 | +- Validate at configure time. Fail loudly on missing required keys. |
| 188 | +- Freeze the config object after the block returns. |
| 189 | +- Provide sensible defaults for every optional setting. |
| 190 | +- Expose `MyGem.configuration` as a frozen reader, never a writer. |
| 191 | + |
| 192 | +--- |
| 193 | + |
| 194 | +## Dependencies |
| 195 | + |
| 196 | +- Minimize runtime dependencies. Each one constrains downstream users. |
| 197 | +- Pin minimum versions (`~> 2.0`). Never pin maximum versions unless a known |
| 198 | + break exists. |
| 199 | +- Lazy-require optional dependencies inside the method that uses them and |
| 200 | + raise a clear error if missing. |
| 201 | +- Development dependencies go in the Gemfile, not the gemspec. |
| 202 | + |
| 203 | +--- |
| 204 | + |
| 205 | +## Idioms to Prefer |
| 206 | + |
| 207 | +- `Array(value)` — nil-safe wrap. |
| 208 | +- `Hash#dig` — nested access without nil checks. |
| 209 | +- `Object#then` / `yield_self` — readable transformations. |
| 210 | +- `Object#tap` — side effects mid-chain. |
| 211 | +- Safe navigation `&.` — one level only. Chains of `&.` hide design |
| 212 | + problems. |
| 213 | +- Pattern matching (`case/in`) for structural conditionals on Ruby 3+. |
| 214 | +- `Set` over `Array#include?` for membership when the collection grows past |
| 215 | + ~10 elements. |
| 216 | +- `String#<<` over `+=` in loops. |
| 217 | +- Heredocs with `<<~` (squiggly) for multiline strings. |
| 218 | +- `each_with_object` over `inject` when accumulating into a mutable |
| 219 | + collection. |
| 220 | +- Memoize with `@x ||= compute` **only when** the value cannot legitimately |
| 221 | + be `nil` or `false`. Otherwise use |
| 222 | + `defined?(@x) ? @x : (@x = compute)`. |
| 223 | + |
| 224 | +--- |
| 225 | + |
| 226 | +## Anti-Patterns (Forbidden) |
| 227 | + |
| 228 | +- Class variables (`@@var`). Use class instance variables or a registry. |
| 229 | +- Global variables (`$var`) outside genuine globals (`$stdout`, `$stderr`). |
| 230 | +- `method_missing` without a paired `respond_to_missing?`. |
| 231 | +- `eval`, `class_eval` with strings, `instance_eval` with strings. |
| 232 | +- `rescue Exception` or bare `rescue`. |
| 233 | +- Rescuing in initializers. |
| 234 | +- `alias_method_chain`-style wrapping. Use `Module#prepend`. |
| 235 | +- Monkey-patching core classes from a published gem. |
| 236 | +- Long parameter lists hidden as `**opts` with no documentation. |
| 237 | +- Returning different shapes from the same method (`String` or `nil` or |
| 238 | + `Array`). Pick one return type and stick to it. |
| 239 | +- `def self.method` scattered through a class. Group under |
| 240 | + `class << self`. |
| 241 | + |
| 242 | +--- |
| 243 | + |
| 244 | +## Documentation |
| 245 | + |
| 246 | +- Every public class, module, and method has a YARD docstring. |
| 247 | +- Document `@param`, `@return`, `@raise`, and provide at least one |
| 248 | + `@example` for non-trivial methods. |
| 249 | +- Keep `README.md` runnable: every snippet must execute against the current |
| 250 | + version. CI should verify this where practical. |
| 251 | +- Maintain `CHANGELOG.md` following the Keep a Changelog format. |
| 252 | +- Document breaking changes prominently and bump major versions. |
| 253 | + |
| 254 | +--- |
| 255 | + |
| 256 | +## Testing |
| 257 | + |
| 258 | +- One test framework per gem. RSpec or Minitest, not both. |
| 259 | +- Test the public API exclusively. Private methods are tested through their |
| 260 | + callers. |
| 261 | +- One logical assertion per test where practical. Group with |
| 262 | + `aggregate_failures` (RSpec) when assertions are about one outcome. |
| 263 | +- Stub external HTTP with WebMock or VCR. |
| 264 | +- Run tests under the lowest and highest supported Ruby versions in CI. |
| 265 | +- No `sleep` in tests. Use proper synchronization or time travel. |
| 266 | + |
| 267 | +--- |
| 268 | + |
| 269 | +## RuboCop Baseline |
| 270 | + |
| 271 | +Ship `.rubocop.yml` with: |
| 272 | + |
| 273 | +```yaml |
| 274 | +AllCops: |
| 275 | + NewCops: enable |
| 276 | + TargetRubyVersion: 3.1 |
| 277 | + SuggestExtensions: false |
| 278 | + |
| 279 | +Layout/LineLength: |
| 280 | + Max: 100 |
| 281 | + |
| 282 | +Metrics/MethodLength: |
| 283 | + Max: 10 |
| 284 | +Metrics/ClassLength: |
| 285 | + Max: 100 |
| 286 | +Metrics/ModuleLength: |
| 287 | + Max: 100 |
| 288 | +Metrics/BlockLength: |
| 289 | + Max: 15 |
| 290 | +Metrics/AbcSize: |
| 291 | + Max: 17 |
| 292 | +Metrics/CyclomaticComplexity: |
| 293 | + Max: 6 |
| 294 | +Metrics/PerceivedComplexity: |
| 295 | + Max: 7 |
| 296 | +Metrics/ParameterLists: |
| 297 | + Max: 4 |
| 298 | + CountKeywordArgs: false |
| 299 | + |
| 300 | +Style/Documentation: |
| 301 | + Enabled: true |
| 302 | +Style/FrozenStringLiteralComment: |
| 303 | + EnforcedStyle: always |
| 304 | +``` |
| 305 | +
|
| 306 | +Treat violations as build failures. Refactor first. Disable a cop only with |
| 307 | +an inline comment justifying the exception. |
| 308 | +
|
| 309 | +--- |
| 310 | +
|
| 311 | +## Reducing Complexity (Refactor Patterns) |
| 312 | +
|
| 313 | +When a method exceeds limits, apply these in order: |
| 314 | +
|
| 315 | +1. **Extract Method.** Pull cohesive lines into a named private method. |
| 316 | +2. **Replace Conditional with Polymorphism.** A long `case` on a type |
| 317 | + becomes classes implementing a shared interface. |
| 318 | +3. **Introduce Parameter Object.** Group related params into `Data` or |
| 319 | + `Struct`. |
| 320 | +4. **Replace Temp with Query.** Turn intermediate variables into methods. |
| 321 | +5. **Decompose Conditional.** Extract the predicate AND each branch into |
| 322 | + named methods. |
| 323 | +6. **Move Method.** If a method uses another object's data more than its |
| 324 | + own, it belongs there. |
| 325 | +7. **Replace Loop with Pipeline.** Chain `map` / `select` / `reduce` |
| 326 | + instead of stateful loops. |
| 327 | +8. **Guard Clauses.** Replace nested `if` with early returns. |
| 328 | + |
| 329 | +If a class breaches 100 lines, look for a second class trying to escape. |
| 330 | +Most overlong classes are hiding a collaborator. Names that signal this: |
| 331 | +`*Manager`, `*Handler`, `*Processor`, `*Helper`, `*Utils`. |
| 332 | + |
| 333 | +If a file breaches 200 lines, the class inside has already breached the |
| 334 | +class limit. Fix the class, the file follows. |
| 335 | + |
| 336 | +--- |
| 337 | + |
| 338 | +## Quick Checklist Before Merge |
| 339 | + |
| 340 | +- [ ] `# frozen_string_literal: true` on every file. |
| 341 | +- [ ] No file > 200 lines, no class > 100, no method > 10. |
| 342 | +- [ ] No line > 100 chars; most lines ≤ 80. |
| 343 | +- [ ] Every public method has a YARD docstring. |
| 344 | +- [ ] Every gem-raised error inherits from `Arcp::Error`. |
| 345 | +- [ ] No `@@`, no bare `rescue`, no `rescue Exception`. |
| 346 | +- [ ] No monkey patches of core classes. |
| 347 | +- [ ] RuboCop exits 0. |
| 348 | +- [ ] All tests pass on min and max supported Ruby. |
| 349 | +- [ ] CHANGELOG updated. Breaking changes flagged. |
0 commit comments